[calligra/krita-psd-rempt] /: Removed explicit memory leak when dealing with gradients

Dmitry Kazakov dimula73 at gmail.com
Sun Apr 12 18:47:14 BST 2015


Git commit 6df0b34f17b742359c197493396e7840f1c5f837 by Dmitry Kazakov.
Committed on 12/04/2015 at 17:46.
Pushed by dkazakov into branch 'krita-psd-rempt'.

Removed explicit memory leak when dealing with gradients

The changes in the patch:

1) Now KoResource has a copy-ctor.

2) KoAbstractGradient and descendants have a clone()
   method to make a copy of the resource.

3) To let KoResourceServer store purely abstract resource types,
   KoResourceServerSimpleContruction utlity-descendant was added.
   It stores the default implementation of createResource() which should
   be overridden in GradientResourceServer later in the hierarchy.

   There was an option to use SFINAE inside KoResourceServer to handle
   this situation for KoAbstractGradient, but the construction is much
   more complicated and not so vastly understood/accepted.

CCMAIL:calligra-devel at kde.org

M  +4    -0    krita/image/kis_gradient_painter.cc
M  +1    -1    krita/plugins/extensions/dockers/tasksetdocker/tasksetdocker_dock.cpp
M  +1    -1    krita/plugins/extensions/resourcemanager/resourcemanager.cpp
M  +1    -1    krita/plugins/filters/layerstyles/kis_asl_layer_style_serializer.cpp
M  +0    -5    krita/plugins/filters/layerstyles/kis_asl_xml_parser.cpp
M  +1    -1    krita/ui/kis_paintop_box.cc
M  +3    -3    krita/ui/kis_resource_server_provider.cpp
M  +1    -1    krita/ui/kis_resource_server_provider.h
M  +6    -0    libs/pigment/resources/KoAbstractGradient.cpp
M  +4    -1    libs/pigment/resources/KoAbstractGradient.h
M  +5    -0    libs/pigment/resources/KoResource.cpp
M  +3    -2    libs/pigment/resources/KoResource.h
M  +5    -0    libs/pigment/resources/KoSegmentGradient.cpp
M  +2    -0    libs/pigment/resources/KoSegmentGradient.h
M  +5    -0    libs/pigment/resources/KoStopGradient.cpp
M  +2    -0    libs/pigment/resources/KoStopGradient.h
M  +15   -1    libs/widgets/KoResourceServer.h
M  +2    -2    libs/widgets/KoResourceServerProvider.cpp

http://commits.kde.org/calligra/6df0b34f17b742359c197493396e7840f1c5f837

diff --git a/krita/image/kis_gradient_painter.cc b/krita/image/kis_gradient_painter.cc
index d23b025..0f29367 100644
--- a/krita/image/kis_gradient_painter.cc
+++ b/krita/image/kis_gradient_painter.cc
@@ -60,6 +60,10 @@ public:
 
     virtual ~CachedGradient() {}
 
+    KoAbstractGradient* clone() const {
+        return new CachedGradient(m_subject, m_max + 1, m_colorSpace);
+    }
+
     /**
     * Creates a QGradient from the gradient.
     * The resulting QGradient might differ from original gradient
diff --git a/krita/plugins/extensions/dockers/tasksetdocker/tasksetdocker_dock.cpp b/krita/plugins/extensions/dockers/tasksetdocker/tasksetdocker_dock.cpp
index e21c8c1..c956f74 100644
--- a/krita/plugins/extensions/dockers/tasksetdocker/tasksetdocker_dock.cpp
+++ b/krita/plugins/extensions/dockers/tasksetdocker/tasksetdocker_dock.cpp
@@ -100,7 +100,7 @@ TasksetDockerDock::TasksetDockerDock( ) : QDockWidget(i18n("Task Sets")), m_canv
     chooserButton->setIcon(koIcon("document-multiple"));
 
     KGlobal::mainComponent().dirs()->addResourceType("kis_taskset", "data", "krita/taskset/");
-    m_rserver = new KoResourceServer<TasksetResource>("kis_taskset", "*.kts");
+    m_rserver = new KoResourceServerSimpleConstruction<TasksetResource>("kis_taskset", "*.kts");
     if (!QFileInfo(m_rserver->saveLocation()).exists()) {
         QDir().mkpath(m_rserver->saveLocation());
     }
diff --git a/krita/plugins/extensions/resourcemanager/resourcemanager.cpp b/krita/plugins/extensions/resourcemanager/resourcemanager.cpp
index c274303..7eb81c3 100644
--- a/krita/plugins/extensions/resourcemanager/resourcemanager.cpp
+++ b/krita/plugins/extensions/resourcemanager/resourcemanager.cpp
@@ -54,7 +54,7 @@ ResourceBundleServerProvider::ResourceBundleServerProvider()
     // user-local
     KGlobal::mainComponent().dirs()->addResourceType("kis_resourcebundles", "data", "krita/bundles/");
     KGlobal::mainComponent().dirs()->addResourceDir("kis_resourcebundles", QDir::homePath() + QString("/.create/bundles"));
-    m_resourceBundleServer = new KoResourceServer<ResourceBundle>("kis_resourcebundles", "*.bundle");
+    m_resourceBundleServer = new KoResourceServerSimpleConstruction<ResourceBundle>("kis_resourcebundles", "*.bundle");
     if (!QFileInfo(m_resourceBundleServer->saveLocation()).exists()) {
         QDir().mkpath(m_resourceBundleServer->saveLocation());
     }
diff --git a/krita/plugins/filters/layerstyles/kis_asl_layer_style_serializer.cpp b/krita/plugins/filters/layerstyles/kis_asl_layer_style_serializer.cpp
index 2d0fb4e..132e13a 100644
--- a/krita/plugins/filters/layerstyles/kis_asl_layer_style_serializer.cpp
+++ b/krita/plugins/filters/layerstyles/kis_asl_layer_style_serializer.cpp
@@ -149,7 +149,7 @@ template <class T>
 void cloneAndSetResource(const T *resource,
                          boost::function<void (T*)> setResource)
 {
-    setResource(const_cast<T *>(resource)/*->clone()*/);
+    setResource(const_cast<T *>(resource)->clone());
 }
 
 inline QString _prepaddr(const QString &addr) {
diff --git a/krita/plugins/filters/layerstyles/kis_asl_xml_parser.cpp b/krita/plugins/filters/layerstyles/kis_asl_xml_parser.cpp
index 921a39f..d99de79 100644
--- a/krita/plugins/filters/layerstyles/kis_asl_xml_parser.cpp
+++ b/krita/plugins/filters/layerstyles/kis_asl_xml_parser.cpp
@@ -468,11 +468,6 @@ bool tryParseDescriptor(const QDomElement &el,
         }
 
         catcher.addGradient(path, gradient.data());
-
-
-        //FIXME: add clone method and fix leaks!
-        gradient.take();
-
     } else {
         retval = false;
     }
diff --git a/krita/ui/kis_paintop_box.cc b/krita/ui/kis_paintop_box.cc
index 60a017e..6f9b38b0 100644
--- a/krita/ui/kis_paintop_box.cc
+++ b/krita/ui/kis_paintop_box.cc
@@ -77,7 +77,7 @@
 
 
 
-typedef KoResourceServer<KisPaintOpPreset, SharedPointerStroragePolicy<KisPaintOpPresetSP> > KisPaintOpPresetResourceServer;
+typedef KoResourceServerSimpleConstruction<KisPaintOpPreset, SharedPointerStroragePolicy<KisPaintOpPresetSP> > KisPaintOpPresetResourceServer;
 typedef KoResourceServerAdapter<KisPaintOpPreset, SharedPointerStroragePolicy<KisPaintOpPresetSP> > KisPaintOpPresetResourceServerAdapter;
 
 KisPaintopBox::KisPaintopBox(KisViewManager *view, QWidget *parent, const char *name)
diff --git a/krita/ui/kis_resource_server_provider.cpp b/krita/ui/kis_resource_server_provider.cpp
index 2116620..8952636 100644
--- a/krita/ui/kis_resource_server_provider.cpp
+++ b/krita/ui/kis_resource_server_provider.cpp
@@ -43,7 +43,7 @@
 
 #include <kis_brush_server.h>
 
-typedef KoResourceServer<KisPaintOpPreset, SharedPointerStroragePolicy<KisPaintOpPresetSP> > KisPaintOpPresetResourceServer;
+typedef KoResourceServerSimpleConstruction<KisPaintOpPreset, SharedPointerStroragePolicy<KisPaintOpPresetSP> > KisPaintOpPresetResourceServer;
 typedef KoResourceServerAdapter<KisPaintOpPreset, SharedPointerStroragePolicy<KisPaintOpPresetSP> > KisPaintOpPresetResourceServerAdapter;
 
 
@@ -73,7 +73,7 @@ KisResourceServerProvider::KisResourceServerProvider()
         m_paintOpPresetThread->barrier();
     }
 
-    m_workspaceServer = new KoResourceServer<KisWorkspaceResource>("kis_workspaces", "*.kws");
+    m_workspaceServer = new KoResourceServerSimpleConstruction<KisWorkspaceResource>("kis_workspaces", "*.kws");
     if (!QFileInfo(m_workspaceServer->saveLocation()).exists()) {
         QDir().mkpath(m_workspaceServer->saveLocation());
     }
@@ -83,7 +83,7 @@ KisResourceServerProvider::KisResourceServerProvider()
         m_workspaceThread->barrier();
     }
 
-    m_layerStyleCollectionServer = new KoResourceServer<KisPSDLayerStyleCollectionResource>("psd_layer_style_collections", "*.asl");
+    m_layerStyleCollectionServer = new KoResourceServerSimpleConstruction<KisPSDLayerStyleCollectionResource>("psd_layer_style_collections", "*.asl");
     if (!QFileInfo(m_layerStyleCollectionServer->saveLocation()).exists()) {
         QDir().mkpath(m_layerStyleCollectionServer->saveLocation());
     }
diff --git a/krita/ui/kis_resource_server_provider.h b/krita/ui/kis_resource_server_provider.h
index 76ba033..e3541e1 100644
--- a/krita/ui/kis_resource_server_provider.h
+++ b/krita/ui/kis_resource_server_provider.h
@@ -42,7 +42,7 @@ class KisPaintOpPreset;
 class KisWorkspaceResource;
 class KisPSDLayerStyleCollectionResource;
 
-typedef KoResourceServer<KisPaintOpPreset, SharedPointerStroragePolicy<KisPaintOpPresetSP> > KisPaintOpPresetResourceServer;
+typedef KoResourceServerSimpleConstruction<KisPaintOpPreset, SharedPointerStroragePolicy<KisPaintOpPresetSP> > KisPaintOpPresetResourceServer;
 typedef KoResourceServerAdapter<KisPaintOpPreset, SharedPointerStroragePolicy<KisPaintOpPresetSP> > KisPaintOpPresetResourceServerAdapter;
 
 class KRITAUI_EXPORT KisResourceServerProvider : public QObject
diff --git a/libs/pigment/resources/KoAbstractGradient.cpp b/libs/pigment/resources/KoAbstractGradient.cpp
index 0f0fa5f..800a490 100644
--- a/libs/pigment/resources/KoAbstractGradient.cpp
+++ b/libs/pigment/resources/KoAbstractGradient.cpp
@@ -51,6 +51,12 @@ KoAbstractGradient::~KoAbstractGradient()
     delete d;
 }
 
+KoAbstractGradient::KoAbstractGradient(const KoAbstractGradient &rhs)
+    : KoResource(rhs)
+    , d(new Private(*rhs.d))
+{
+}
+
 void KoAbstractGradient::colorAt(KoColor&, qreal t) const
 {
     Q_UNUSED(t);
diff --git a/libs/pigment/resources/KoAbstractGradient.h b/libs/pigment/resources/KoAbstractGradient.h
index d6d6d4f..913a7a1 100644
--- a/libs/pigment/resources/KoAbstractGradient.h
+++ b/libs/pigment/resources/KoAbstractGradient.h
@@ -31,11 +31,12 @@ class KoColor;
  */
 class PIGMENTCMS_EXPORT KoAbstractGradient : public KoResource
 {
-
 public:
     explicit KoAbstractGradient(const QString &filename);
     virtual ~KoAbstractGradient();
 
+    virtual KoAbstractGradient* clone() const = 0;
+
     virtual bool load() {
         return false;
     }
@@ -78,6 +79,8 @@ public:
 protected:
     virtual QByteArray generateMD5() const;
 
+    KoAbstractGradient(const KoAbstractGradient &rhs);
+
 private:
     struct Private;
     Private* const d;
diff --git a/libs/pigment/resources/KoResource.cpp b/libs/pigment/resources/KoResource.cpp
index d64c460..6443399 100644
--- a/libs/pigment/resources/KoResource.cpp
+++ b/libs/pigment/resources/KoResource.cpp
@@ -47,6 +47,11 @@ KoResource::~KoResource()
     delete d;
 }
 
+KoResource::KoResource(const KoResource &rhs)
+    : d(new Private(*rhs.d))
+{
+}
+
 QImage KoResource::image() const
 {
     return d->image;
diff --git a/libs/pigment/resources/KoResource.h b/libs/pigment/resources/KoResource.h
index e5ac7aa..b2c0fc8 100644
--- a/libs/pigment/resources/KoResource.h
+++ b/libs/pigment/resources/KoResource.h
@@ -105,12 +105,13 @@ protected:
     /// call this when the contents of the resource change so the md5 needs to be recalculated
     void setMD5(const QByteArray &md5);
 
+protected:
+    KoResource(const KoResource &rhs);
+
 private:
     /// save the resource as XML to the given document with the given element as root
     virtual void toXML(QDomDocument& doc, QDomElement& element) const;
 
-    Q_DISABLE_COPY(KoResource)
-
 private:
     struct Private;
     Private* const d;
diff --git a/libs/pigment/resources/KoSegmentGradient.cpp b/libs/pigment/resources/KoSegmentGradient.cpp
index 48bf563..22438c5 100644
--- a/libs/pigment/resources/KoSegmentGradient.cpp
+++ b/libs/pigment/resources/KoSegmentGradient.cpp
@@ -61,6 +61,11 @@ KoSegmentGradient::~KoSegmentGradient()
     }
 }
 
+KoAbstractGradient* KoSegmentGradient::clone() const
+{
+    return new KoSegmentGradient(*this);
+}
+
 bool KoSegmentGradient::load()
 {
     QFile file(filename());
diff --git a/libs/pigment/resources/KoSegmentGradient.h b/libs/pigment/resources/KoSegmentGradient.h
index 42cded3..99125d8 100644
--- a/libs/pigment/resources/KoSegmentGradient.h
+++ b/libs/pigment/resources/KoSegmentGradient.h
@@ -263,6 +263,8 @@ public:
     explicit KoSegmentGradient(const QString &file);
     virtual ~KoSegmentGradient();
 
+    KoAbstractGradient* clone() const;
+
     /// reimplemented
     virtual bool load();
     virtual bool loadFromDevice(QIODevice *dev);
diff --git a/libs/pigment/resources/KoStopGradient.cpp b/libs/pigment/resources/KoStopGradient.cpp
index 83ad3bf..3bd045b 100644
--- a/libs/pigment/resources/KoStopGradient.cpp
+++ b/libs/pigment/resources/KoStopGradient.cpp
@@ -46,6 +46,11 @@ KoStopGradient::~KoStopGradient()
 {
 }
 
+KoAbstractGradient* KoStopGradient::clone() const
+{
+    return new KoStopGradient(*this);
+}
+
 bool KoStopGradient::load()
 {
     QFile f(filename());
diff --git a/libs/pigment/resources/KoStopGradient.h b/libs/pigment/resources/KoStopGradient.h
index cb069cf..a520e56 100644
--- a/libs/pigment/resources/KoStopGradient.h
+++ b/libs/pigment/resources/KoStopGradient.h
@@ -39,6 +39,8 @@ public:
     explicit KoStopGradient(const QString &filename);
     virtual ~KoStopGradient();
 
+    KoAbstractGradient* clone() const;
+
     virtual bool load();
     virtual bool loadFromDevice(QIODevice *dev);
     virtual bool save();
diff --git a/libs/widgets/KoResourceServer.h b/libs/widgets/KoResourceServer.h
index f9cde44..0adefd6 100644
--- a/libs/widgets/KoResourceServer.h
+++ b/libs/widgets/KoResourceServer.h
@@ -559,7 +559,7 @@ public:
         return createdResources;
     }
 
-    virtual PointerType createResource( const QString & filename ) { return new T(filename); }
+    virtual PointerType createResource( const QString & filename ) = 0;
 
     /// Return the currently stored resources in alphabetical order, overwrite for customized sorting
     virtual QList<PointerType> sortedResources()
@@ -698,4 +698,18 @@ private:
 
 };
 
+template <class T, class Policy = PointerStroragePolicy<T> >
+    class KoResourceServerSimpleConstruction : public KoResourceServer<T, Policy>
+{
+public:
+    KoResourceServerSimpleConstruction(const QString& type, const QString& extensions)
+: KoResourceServer<T, Policy>(type, extensions)
+    {
+    }
+
+typename KoResourceServer<T, Policy>::PointerType createResource( const QString & filename ) {
+        return new T(filename);
+    }
+};
+
 #endif // KORESOURCESERVER_H
diff --git a/libs/widgets/KoResourceServerProvider.cpp b/libs/widgets/KoResourceServerProvider.cpp
index 872a19b..995827a 100644
--- a/libs/widgets/KoResourceServerProvider.cpp
+++ b/libs/widgets/KoResourceServerProvider.cpp
@@ -177,7 +177,7 @@ KoResourceServerProvider::KoResourceServerProvider() : d(new Private)
     KGlobal::mainComponent().dirs()->addResourceDir("ko_palettes", "/usr/share/create/swatches");
     KGlobal::mainComponent().dirs()->addResourceDir("ko_palettes", QDir::homePath() + QString("/.create/swatches"));
 
-    d->patternServer = new KoResourceServer<KoPattern>("ko_patterns", "*.pat:*.jpg:*.gif:*.png:*.tif:*.xpm:*.bmp" );
+    d->patternServer = new KoResourceServerSimpleConstruction<KoPattern>("ko_patterns", "*.pat:*.jpg:*.gif:*.png:*.tif:*.xpm:*.bmp" );
     if (!QFileInfo(d->patternServer->saveLocation()).exists()) {
         QDir().mkpath(d->patternServer->saveLocation());
     }
@@ -199,7 +199,7 @@ KoResourceServerProvider::KoResourceServerProvider() : d(new Private)
         d->gradientThread->wait();
     }
 
-    d->paletteServer = new KoResourceServer<KoColorSet>("ko_palettes", "*.gpl:*.pal:*.act:*.aco:*.css:*.colors");
+    d->paletteServer = new KoResourceServerSimpleConstruction<KoColorSet>("ko_palettes", "*.gpl:*.pal:*.act:*.aco:*.css:*.colors");
     if (!QFileInfo(d->paletteServer->saveLocation()).exists()) {
         QDir().mkpath(d->paletteServer->saveLocation());
     }



More information about the calligra-devel mailing list