Skip to content

Commit

Permalink
ENH: Improve plugin factories
Browse files Browse the repository at this point in the history
- Remove public setInstance function and do the cleanup in a private cleanup function
- Add PythonQtDecorator classes to be able to use the instance function as static in Python to create the factory class
- Make constructor and destructor private to prevent accidental wrong use (add PythonQtWrapper generated class as friend to allow this)
- Update usage of these factories that used the workaround to fix non-static instance function

Re commontk/CTK#970
  • Loading branch information
cpinter committed May 28, 2021
1 parent daad7c2 commit ec36be5
Show file tree
Hide file tree
Showing 10 changed files with 177 additions and 74 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class qSlicerMarkupsAdditionalOptionsWidgetsFactoryCleanup
{
if (qSlicerMarkupsAdditionalOptionsWidgetsFactory::Instance)
{
qSlicerMarkupsAdditionalOptionsWidgetsFactory::setInstance(nullptr);
qSlicerMarkupsAdditionalOptionsWidgetsFactory::cleanup();
}
}
};
Expand All @@ -59,23 +59,12 @@ qSlicerMarkupsAdditionalOptionsWidgetsFactory* qSlicerMarkupsAdditionalOptionsWi
}

//-----------------------------------------------------------------------------
void qSlicerMarkupsAdditionalOptionsWidgetsFactory::setInstance(qSlicerMarkupsAdditionalOptionsWidgetsFactory* instance)
void qSlicerMarkupsAdditionalOptionsWidgetsFactory::cleanup()
{
if (qSlicerMarkupsAdditionalOptionsWidgetsFactory::Instance==instance)
{
return;
}

// Preferably this will be nullptr
if (qSlicerMarkupsAdditionalOptionsWidgetsFactory::Instance)
{
delete qSlicerMarkupsAdditionalOptionsWidgetsFactory::Instance;
}

qSlicerMarkupsAdditionalOptionsWidgetsFactory::Instance = instance;
if (!instance)
{
return;
qSlicerMarkupsAdditionalOptionsWidgetsFactory::Instance = nullptr;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,6 @@ class Q_SLICER_MODULE_MARKUPS_WIDGETS_EXPORT qSlicerMarkupsAdditionalOptionsWidg
/// \return Instance object
Q_INVOKABLE static qSlicerMarkupsAdditionalOptionsWidgetsFactory* instance();

/// Allows cleanup of the singleton at application exit
static void setInstance(qSlicerMarkupsAdditionalOptionsWidgetsFactory* instance);

public:
/// Registers an additional options widget.
Q_INVOKABLE bool registerAdditionalOptionsWidget(qSlicerMarkupsAdditionalOptionsWidget* widget);
Expand All @@ -67,17 +64,17 @@ class Q_SLICER_MODULE_MARKUPS_WIDGETS_EXPORT qSlicerMarkupsAdditionalOptionsWidg
protected:
QList<QPointer<qSlicerMarkupsAdditionalOptionsWidget>> AdditionalOptionsWidgets;

public:
/// Private constructor made public to enable python wrapping
/// IMPORTANT: Should not be used for creating effect handler! Use instance() instead.
qSlicerMarkupsAdditionalOptionsWidgetsFactory(QObject* parent=nullptr);
private:
/// Allows cleanup of the singleton at application exit
static void cleanup();

/// Private destructor made public to enable python wrapping
private:
qSlicerMarkupsAdditionalOptionsWidgetsFactory(QObject* parent=nullptr);
~qSlicerMarkupsAdditionalOptionsWidgetsFactory() override;

private:
Q_DISABLE_COPY(qSlicerMarkupsAdditionalOptionsWidgetsFactory);
friend class qSlicerMarkupsAdditionalOptionsWidgetsFactoryCleanup;
friend class PythonQtWrapper_qSlicerMarkupsAdditionalOptionsWidgetsFactory; // Allow Python wrapping without enabling direct instantiation

private:
/// Instance of the singleton
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/*=auto=========================================================================
Portions (c) Copyright 2005 Brigham and Women's Hospital (BWH)
All Rights Reserved.
See COPYRIGHT.txt
or http://www.slicer.org/copyright/copyright.txt for details.
Program: 3D Slicer
=========================================================================auto=*/

#ifndef __qSlicerMarkupsModuleWidgetsPythonQtDecorators_h
#define __qSlicerMarkupsModuleWidgetsPythonQtDecorators_h

// PythonQt includes
#include <PythonQt.h>

// Slicer includes
#include "qSlicerMarkupsAdditionalOptionsWidgetsFactory.h"

#include "qSlicerMarkupsModuleWidgetsExport.h"

// NOTE:
//
// For decorators it is assumed that the methods will never be called
// with the self argument as nullptr. The self argument is the first argument
// for non-static methods.
//

class qSlicerMarkupsModuleWidgetsPythonQtDecorators : public QObject
{
Q_OBJECT
public:

qSlicerMarkupsModuleWidgetsPythonQtDecorators()
{
//PythonQt::self()->registerClass(&qSlicerMarkupsAdditionalOptionsWidgetsFactory::staticMetaObject);
// Note: Use registerCPPClassForPythonQt to register pure Cpp classes
}

public slots:

//----------------------------------------------------------------------------
// qSlicerMarkupsAdditionalOptionsWidgetsFactory

//----------------------------------------------------------------------------
// static methods

//----------------------------------------------------------------------------
qSlicerMarkupsAdditionalOptionsWidgetsFactory* static_qSlicerMarkupsAdditionalOptionsWidgetsFactory_instance()
{
return qSlicerMarkupsAdditionalOptionsWidgetsFactory::instance();
}

//----------------------------------------------------------------------------
// instance methods

//----------------------------------------------------------------------------
bool registerAdditionalOptionsWidget(qSlicerMarkupsAdditionalOptionsWidgetsFactory* factory,
PythonQtPassOwnershipToCPP<qSlicerMarkupsAdditionalOptionsWidget*> plugin)
{
return factory->registerAdditionalOptionsWidget(plugin);
}
};

//-----------------------------------------------------------------------------
void initqSlicerMarkupsModuleWidgetsPythonQtDecorators()
{
PythonQt::self()->addDecorators(new qSlicerMarkupsModuleWidgetsPythonQtDecorators);
}

#endif
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,7 @@ def __init__(self, scriptedEffect):
self.scriptedEffect = scriptedEffect

def register(self):
import qSlicerSegmentationsEditorEffectsPythonQt
#TODO: For some reason the instance() function cannot be called as a class function although it's static
factory = qSlicerSegmentationsEditorEffectsPythonQt.qSlicerSegmentEditorEffectFactory()
effectFactorySingleton = factory.instance()
effectFactorySingleton = slicer.qSlicerSegmentEditorEffectFactory.instance()
effectFactorySingleton.registerEffect(self.scriptedEffect)

#
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class qSlicerSegmentEditorEffectFactoryCleanup
{
if (qSlicerSegmentEditorEffectFactory::m_Instance)
{
qSlicerSegmentEditorEffectFactory::setInstance(nullptr);
qSlicerSegmentEditorEffectFactory::cleanup();
}
}
};
Expand All @@ -60,21 +60,12 @@ qSlicerSegmentEditorEffectFactory* qSlicerSegmentEditorEffectFactory::instance()
}

//-----------------------------------------------------------------------------
void qSlicerSegmentEditorEffectFactory::setInstance(qSlicerSegmentEditorEffectFactory* instance)
void qSlicerSegmentEditorEffectFactory::cleanup()
{
if (qSlicerSegmentEditorEffectFactory::m_Instance==instance)
{
return;
}
// Preferably this will be nullptr
if (qSlicerSegmentEditorEffectFactory::m_Instance)
{
delete qSlicerSegmentEditorEffectFactory::m_Instance;
}
qSlicerSegmentEditorEffectFactory::m_Instance = instance;
if (!instance)
{
return;
qSlicerSegmentEditorEffectFactory::m_Instance = nullptr;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,6 @@ class Q_SLICER_SEGMENTATIONS_EFFECTS_EXPORT qSlicerSegmentEditorEffectFactory :
/// \return Instance object
Q_INVOKABLE static qSlicerSegmentEditorEffectFactory* instance();

/// Allows cleanup of the singleton at application exit
static void setInstance(qSlicerSegmentEditorEffectFactory* instance);

public:
/// Register a effect
/// \return True if effect registered successfully, false otherwise
Expand All @@ -69,17 +66,17 @@ class Q_SLICER_SEGMENTATIONS_EFFECTS_EXPORT qSlicerSegmentEditorEffectFactory :
/// List of registered effect instances
QList<qSlicerSegmentEditorAbstractEffect*> m_RegisteredEffects;

public:
/// Private constructor made public to enable python wrapping
/// IMPORTANT: Should not be used for creating effect handler! Use instance() instead.
qSlicerSegmentEditorEffectFactory(QObject* parent=nullptr);
private:
/// Allows cleanup of the singleton at application exit
static void cleanup();

/// Private destructor made public to enable python wrapping
private:
qSlicerSegmentEditorEffectFactory(QObject* parent=nullptr);
~qSlicerSegmentEditorEffectFactory() override;

private:
Q_DISABLE_COPY(qSlicerSegmentEditorEffectFactory);
friend class qSlicerSegmentEditorEffectFactoryCleanup;
friend class PythonQtWrapper_qSlicerSegmentEditorEffectFactory; // Allow Python wrapping without enabling direct instantiation

private:
/// Instance of the singleton
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/*=auto=========================================================================
Portions (c) Copyright 2005 Brigham and Women's Hospital (BWH)
All Rights Reserved.
See COPYRIGHT.txt
or http://www.slicer.org/copyright/copyright.txt for details.
Program: 3D Slicer
=========================================================================auto=*/

#ifndef __qSlicerSegmentationsEditorEffectsPythonQtDecorators_h
#define __qSlicerSegmentationsEditorEffectsPythonQtDecorators_h

// PythonQt includes
#include <PythonQt.h>

// Slicer includes
#include "qSlicerSegmentEditorEffectFactory.h"

#include "qSlicerSegmentationsEditorEffectsExport.h"

// NOTE:
//
// For decorators it is assumed that the methods will never be called
// with the self argument as nullptr. The self argument is the first argument
// for non-static methods.
//

class qSlicerSegmentationsEditorEffectsPythonQtDecorators : public QObject
{
Q_OBJECT
public:

qSlicerSegmentationsEditorEffectsPythonQtDecorators()
{
//PythonQt::self()->registerClass(&qSlicerSegmentEditorEffectFactory::staticMetaObject);
// Note: Use registerCPPClassForPythonQt to register pure Cpp classes
}

public slots:

//----------------------------------------------------------------------------
// qSlicerSegmentEditorEffectFactory

//----------------------------------------------------------------------------
// static methods

//----------------------------------------------------------------------------
qSlicerSegmentEditorEffectFactory* static_qSlicerSegmentEditorEffectFactory_instance()
{
return qSlicerSegmentEditorEffectFactory::instance();
}

//----------------------------------------------------------------------------
// instance methods

//----------------------------------------------------------------------------
bool registerEffect(qSlicerSegmentEditorEffectFactory* factory,
PythonQtPassOwnershipToCPP<qSlicerSegmentEditorAbstractEffect*> plugin)
{
return factory->registerEffect(plugin);
}
};

//-----------------------------------------------------------------------------
void initqSlicerSegmentationsEditorEffectsPythonQtDecorators()
{
PythonQt::self()->addDecorators(new qSlicerSegmentationsEditorEffectsPythonQtDecorators);
}

#endif
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class qSlicerSubjectHierarchyPluginHandlerCleanup
{
if (qSlicerSubjectHierarchyPluginHandler::m_Instance)
{
qSlicerSubjectHierarchyPluginHandler::setInstance(nullptr);
qSlicerSubjectHierarchyPluginHandler::cleanup();
}
}
};
Expand All @@ -74,21 +74,12 @@ qSlicerSubjectHierarchyPluginHandler* qSlicerSubjectHierarchyPluginHandler::inst
}

//-----------------------------------------------------------------------------
void qSlicerSubjectHierarchyPluginHandler::setInstance(qSlicerSubjectHierarchyPluginHandler* instance)
void qSlicerSubjectHierarchyPluginHandler::cleanup()
{
if (qSlicerSubjectHierarchyPluginHandler::m_Instance==instance)
{
return;
}
// Preferably this will be nullptr
if (qSlicerSubjectHierarchyPluginHandler::m_Instance)
{
delete qSlicerSubjectHierarchyPluginHandler::m_Instance;
}
qSlicerSubjectHierarchyPluginHandler::m_Instance = instance;
if (!instance)
{
return;
qSlicerSubjectHierarchyPluginHandler::m_Instance = nullptr;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,6 @@ class Q_SLICER_MODULE_SUBJECTHIERARCHY_WIDGETS_EXPORT qSlicerSubjectHierarchyPlu
/// \return Instance object
Q_INVOKABLE static qSlicerSubjectHierarchyPluginHandler* instance();

/// Allows cleanup of the singleton at application exit
static void setInstance(qSlicerSubjectHierarchyPluginHandler* instance);

/// Get subject hierarchy node
Q_INVOKABLE vtkMRMLSubjectHierarchyNode* subjectHierarchyNode()const;
/// Set MRML scene
Expand Down Expand Up @@ -226,19 +223,20 @@ class Q_SLICER_MODULE_SUBJECTHIERARCHY_WIDGETS_EXPORT qSlicerSubjectHierarchyPlu
vtkSmartPointer<vtkCallbackCommand> m_CallBack;

public:
/// Private constructor made public to enable python wrapping
/// IMPORTANT: Should not be used for creating plugin handler! Use instance() instead.
qSlicerSubjectHierarchyPluginHandler(QObject* parent=nullptr);

/// Private destructor made public to enable python wrapping
~qSlicerSubjectHierarchyPluginHandler() override;

/// Timestamp of the last plugin registration. Used to allow context menus be repopulated if needed.
QDateTime LastPluginRegistrationTime;

private:
/// Allows cleanup of the singleton at application exit
static void cleanup();

private:
qSlicerSubjectHierarchyPluginHandler(QObject* parent=nullptr);
~qSlicerSubjectHierarchyPluginHandler() override;

Q_DISABLE_COPY(qSlicerSubjectHierarchyPluginHandler);
friend class qSlicerSubjectHierarchyPluginHandlerCleanup;
friend class PythonQtWrapper_qSlicerSubjectHierarchyPluginHandler; // Allow Python wrapping without enabling direct instantiation

private:
/// Instance of the singleton
Expand Down
5 changes: 1 addition & 4 deletions Modules/Scripted/SegmentEditor/SegmentEditor.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,7 @@ def setup(self):
# Observe editor effect registrations to make sure that any effects that are registered
# later will show up in the segment editor widget. For example, if Segment Editor is set
# as startup module, additional effects are registered after the segment editor widget is created.
import qSlicerSegmentationsEditorEffectsPythonQt
#TODO: For some reason the instance() function cannot be called as a class function although it's static
factory = qSlicerSegmentationsEditorEffectsPythonQt.qSlicerSegmentEditorEffectFactory()
self.effectFactorySingleton = factory.instance()
self.effectFactorySingleton = slicer.qSlicerSegmentEditorEffectFactory.instance()
self.effectFactorySingleton.connect('effectRegistered(QString)', self.editorEffectRegistered)

# Connect observers to scene events
Expand Down

0 comments on commit ec36be5

Please sign in to comment.