Skip to content

Commit

Permalink
Use /TransferBack/ instead of /Factory/ in layout registry factory me…
Browse files Browse the repository at this point in the history 
…thods

For same reason as we do in Processing registry:

(from the comments included in this commit)
"
While it seems like /Factory/ would be the correct annotations here, that's not
the case.
As per Phil Thomson's advice on https://www.riverbankcomputing.com/pipermail/pyqt/2017-July/039450.html:

"
/Factory/ is used when the instance returned is guaranteed to be new to Python.
In this case it isn't because it has already been seen when being returned by QgsProcessingAlgorithm::createInstance()
(However for a different sub-class implemented in C++ then it would be the first time it was seen
by Python so the /Factory/ on create() would be correct.)

You might try using /TransferBack/ on create() instead - that might be the best compromise.
"
  • Loading branch information
@nyalldawson
nyalldawson committed Dec 21, 2020
1 parent b7a7389 commit da59316
Show file tree
Hide file tree
Showing 5 changed files with 117 additions and 21 deletions.
11 changes: 7 additions & 4 deletions python/core/auto_generated/layout/qgslayoutitemregistry.sip.in
View file
Expand Up @@ -58,7 +58,8 @@ Returns a translated, user visible name for plurals of the layout item class (e.
.. versionadded:: 3.10
%End

virtual QgsLayoutItem *createItem( QgsLayout *layout ) = 0 /Factory/;

virtual QgsLayoutItem *createItem( QgsLayout *layout ) = 0 /TransferBack/;
%Docstring
Creates a layout item of this class for a specified ``layout``.
%End
Expand Down Expand Up @@ -120,7 +121,8 @@ Returns an icon representing the layout multiframe type.
Returns a translated, user visible name for the layout multiframe class.
%End

virtual QgsLayoutMultiFrame *createMultiFrame( QgsLayout *layout ) = 0 /Factory/;

virtual QgsLayoutMultiFrame *createMultiFrame( QgsLayout *layout ) = 0 /TransferBack/;
%Docstring
Creates a layout multiframe of this class for a specified ``layout``.
%End
Expand Down Expand Up @@ -233,6 +235,7 @@ a corresponding type was not found in the registry.
.. seealso:: :py:func:`itemMetadata`
%End


bool addLayoutItemType( QgsLayoutItemAbstractMetadata *metadata /Transfer/ );
%Docstring
Registers a new layout item type. Takes ownership of the metadata instance.
Expand All @@ -247,14 +250,14 @@ Registers a new layout multiframe type. Takes ownership of the metadata instance
.. seealso:: :py:func:`addLayoutItemType`
%End

QgsLayoutItem *createItem( int type, QgsLayout *layout ) const /Factory/;
QgsLayoutItem *createItem( int type, QgsLayout *layout ) const /TransferBack/;
%Docstring
Creates a new instance of a layout item given the item ``type``, and target ``layout``.

.. seealso:: :py:func:`createMultiFrame`
%End

QgsLayoutMultiFrame *createMultiFrame( int type, QgsLayout *layout ) const /Factory/;
QgsLayoutMultiFrame *createMultiFrame( int type, QgsLayout *layout ) const /TransferBack/;
%Docstring
Creates a new instance of a layout multiframe given the multiframe ``type``, and target ``layout``.

Expand Down
15 changes: 9 additions & 6 deletions python/gui/auto_generated/layout/qgslayoutitemguiregistry.sip.in
View file
Expand Up @@ -79,28 +79,29 @@ Returns a translated, user visible name identifying the corresponding layout ite
Returns an icon representing creation of the layout item type.
%End

virtual QgsLayoutItemBaseWidget *createItemWidget( QgsLayoutItem *item ) /Factory/;

virtual QgsLayoutItemBaseWidget *createItemWidget( QgsLayoutItem *item ) /TransferBack/;
%Docstring
Creates a configuration widget for an ``item`` of this type. Can return ``None`` if no configuration GUI is required.
%End

virtual QgsLayoutViewRubberBand *createRubberBand( QgsLayoutView *view ) /Factory/;
virtual QgsLayoutViewRubberBand *createRubberBand( QgsLayoutView *view ) /TransferBack/;
%Docstring
Creates a rubber band for use when creating layout items of this type. Can return ``None`` if no rubber band
should be created. The default behavior is to create a rectangular rubber band.

.. seealso:: :py:func:`createNodeRubberBand`
%End

virtual QAbstractGraphicsShapeItem *createNodeRubberBand( QgsLayoutView *view ) /Factory/;
virtual QAbstractGraphicsShapeItem *createNodeRubberBand( QgsLayoutView *view ) /TransferBack/;
%Docstring
Creates a rubber band for use when creating layout node based items of this type. Can return ``None`` if no rubber band
should be created. The default behavior is to return ``None``.

.. seealso:: :py:func:`createRubberBand`
%End

virtual QgsLayoutItem *createItem( QgsLayout *layout ) /Factory/;
virtual QgsLayoutItem *createItem( QgsLayout *layout ) /TransferBack/;
%Docstring
Creates an instance of the corresponding item type.
%End
Expand Down Expand Up @@ -212,7 +213,8 @@ Returns a reference to the item group with matching ``id``.
.. seealso:: :py:func:`addItemGroup`
%End

QgsLayoutItem *createItem( int metadataId, QgsLayout *layout ) const /Factory/;

QgsLayoutItem *createItem( int metadataId, QgsLayout *layout ) const /TransferBack/;
%Docstring
Creates a new instance of a layout item given the item metadata ``metadataId``, target ``layout``.
%End
Expand All @@ -225,7 +227,8 @@ This is only called for additions which result from GUI operations - i.e. it is
called for items added from templates.
%End

QgsLayoutItemBaseWidget *createItemWidget( QgsLayoutItem *item ) const /Factory/;

QgsLayoutItemBaseWidget *createItemWidget( QgsLayoutItem *item ) const /TransferBack/;
%Docstring
Creates a new instance of a layout item configuration widget for the specified ``item``.
%End
Expand Down
53 changes: 49 additions & 4 deletions src/core/layout/qgslayoutitemregistry.h
View file
Expand Up @@ -77,10 +77,25 @@ class CORE_EXPORT QgsLayoutItemAbstractMetadata
*/
QString visiblePluralName() const { return mVisibleNamePlural; }

/*
* IMPORTANT: While it seems like /Factory/ would be the correct annotations here, that's not
* the case.
* As per Phil Thomson's advice on https://www.riverbankcomputing.com/pipermail/pyqt/2017-July/039450.html:
*
* "
* /Factory/ is used when the instance returned is guaranteed to be new to Python.
* In this case it isn't because it has already been seen when being returned by QgsProcessingAlgorithm::createInstance()
* (However for a different sub-class implemented in C++ then it would be the first time it was seen
* by Python so the /Factory/ on create() would be correct.)
*
* You might try using /TransferBack/ on create() instead - that might be the best compromise.
* "
*/

/**
* Creates a layout item of this class for a specified \a layout.
*/
virtual QgsLayoutItem *createItem( QgsLayout *layout ) = 0 SIP_FACTORY;
virtual QgsLayoutItem *createItem( QgsLayout *layout ) = 0 SIP_TRANSFERBACK;

/**
* Resolve paths in the item's \a properties (if there are any paths).
Expand Down Expand Up @@ -201,10 +216,25 @@ class CORE_EXPORT QgsLayoutMultiFrameAbstractMetadata
*/
QString visibleName() const { return mVisibleName; }

/*
* IMPORTANT: While it seems like /Factory/ would be the correct annotations here, that's not
* the case.
* As per Phil Thomson's advice on https://www.riverbankcomputing.com/pipermail/pyqt/2017-July/039450.html:
*
* "
* /Factory/ is used when the instance returned is guaranteed to be new to Python.
* In this case it isn't because it has already been seen when being returned by QgsProcessingAlgorithm::createInstance()
* (However for a different sub-class implemented in C++ then it would be the first time it was seen
* by Python so the /Factory/ on create() would be correct.)
*
* You might try using /TransferBack/ on create() instead - that might be the best compromise.
* "
*/

/**
* Creates a layout multiframe of this class for a specified \a layout.
*/
virtual QgsLayoutMultiFrame *createMultiFrame( QgsLayout *layout ) = 0 SIP_FACTORY;
virtual QgsLayoutMultiFrame *createMultiFrame( QgsLayout *layout ) = 0 SIP_TRANSFERBACK;

/**
* Resolve paths in the item's \a properties (if there are any paths).
Expand Down Expand Up @@ -379,6 +409,21 @@ class CORE_EXPORT QgsLayoutItemRegistry : public QObject
*/
QgsLayoutMultiFrameAbstractMetadata *multiFrameMetadata( int type ) const;

/*
* IMPORTANT: While it seems like /Factory/ would be the correct annotations here, that's not
* the case.
* As per Phil Thomson's advice on https://www.riverbankcomputing.com/pipermail/pyqt/2017-July/039450.html:
*
* "
* /Factory/ is used when the instance returned is guaranteed to be new to Python.
* In this case it isn't because it has already been seen when being returned by QgsProcessingAlgorithm::createInstance()
* (However for a different sub-class implemented in C++ then it would be the first time it was seen
* by Python so the /Factory/ on create() would be correct.)
*
* You might try using /TransferBack/ on create() instead - that might be the best compromise.
* "
*/

/**
* Registers a new layout item type. Takes ownership of the metadata instance.
* \see addLayoutMultiFrameType()
Expand All @@ -395,13 +440,13 @@ class CORE_EXPORT QgsLayoutItemRegistry : public QObject
* Creates a new instance of a layout item given the item \a type, and target \a layout.
* \see createMultiFrame()
*/
QgsLayoutItem *createItem( int type, QgsLayout *layout ) const SIP_FACTORY;
QgsLayoutItem *createItem( int type, QgsLayout *layout ) const SIP_TRANSFERBACK;

/**
* Creates a new instance of a layout multiframe given the multiframe \a type, and target \a layout.
* \see createItem()
*/
QgsLayoutMultiFrame *createMultiFrame( int type, QgsLayout *layout ) const SIP_FACTORY;
QgsLayoutMultiFrame *createMultiFrame( int type, QgsLayout *layout ) const SIP_TRANSFERBACK;

/**
* Resolve paths in properties of a particular symbol layer.
Expand Down
2 changes: 1 addition & 1 deletion src/core/layout/qgslayoutitemundocommand.h
View file
Expand Up @@ -70,7 +70,7 @@ class CORE_EXPORT QgsLayoutItemUndoCommand: public QgsAbstractLayoutUndoCommand
void saveState( QDomDocument &stateDoc ) const override;
void restoreState( QDomDocument &stateDoc ) override;

virtual QgsLayoutItem *recreateItem( int itemType, QgsLayout *layout ) SIP_FACTORY;
virtual QgsLayoutItem *recreateItem( int itemType, QgsLayout *layout );

private:

Expand Down
57 changes: 51 additions & 6 deletions src/gui/layout/qgslayoutitemguiregistry.h
View file
Expand Up @@ -103,29 +103,44 @@ class GUI_EXPORT QgsLayoutItemAbstractGuiMetadata
*/
virtual QIcon creationIcon() const { return QgsApplication::getThemeIcon( QStringLiteral( "/mActionAddBasicRectangle.svg" ) ); }

/*
* IMPORTANT: While it seems like /Factory/ would be the correct annotations here, that's not
* the case.
* As per Phil Thomson's advice on https://www.riverbankcomputing.com/pipermail/pyqt/2017-July/039450.html:
*
* "
* /Factory/ is used when the instance returned is guaranteed to be new to Python.
* In this case it isn't because it has already been seen when being returned by QgsProcessingAlgorithm::createInstance()
* (However for a different sub-class implemented in C++ then it would be the first time it was seen
* by Python so the /Factory/ on create() would be correct.)
*
* You might try using /TransferBack/ on create() instead - that might be the best compromise.
* "
*/

/**
* Creates a configuration widget for an \a item of this type. Can return NULLPTR if no configuration GUI is required.
*/
virtual QgsLayoutItemBaseWidget *createItemWidget( QgsLayoutItem *item ) SIP_FACTORY { Q_UNUSED( item ) return nullptr; }
virtual QgsLayoutItemBaseWidget *createItemWidget( QgsLayoutItem *item ) SIP_TRANSFERBACK { Q_UNUSED( item ) return nullptr; }

/**
* Creates a rubber band for use when creating layout items of this type. Can return NULLPTR if no rubber band
* should be created. The default behavior is to create a rectangular rubber band.
* \see createNodeRubberBand()
*/
virtual QgsLayoutViewRubberBand *createRubberBand( QgsLayoutView *view ) SIP_FACTORY;
virtual QgsLayoutViewRubberBand *createRubberBand( QgsLayoutView *view ) SIP_TRANSFERBACK;

/**
* Creates a rubber band for use when creating layout node based items of this type. Can return NULLPTR if no rubber band
* should be created. The default behavior is to return NULLPTR.
* \see createRubberBand()
*/
virtual QAbstractGraphicsShapeItem *createNodeRubberBand( QgsLayoutView *view ) SIP_FACTORY;
virtual QAbstractGraphicsShapeItem *createNodeRubberBand( QgsLayoutView *view ) SIP_TRANSFERBACK;

/**
* Creates an instance of the corresponding item type.
*/
virtual QgsLayoutItem *createItem( QgsLayout *layout ) SIP_FACTORY;
virtual QgsLayoutItem *createItem( QgsLayout *layout ) SIP_TRANSFERBACK;

/**
* Called when a newly created item of the associated type has been added to a layout.
Expand Down Expand Up @@ -377,10 +392,25 @@ class GUI_EXPORT QgsLayoutItemGuiRegistry : public QObject
*/
const QgsLayoutItemGuiGroup &itemGroup( const QString &id );

/*
* IMPORTANT: While it seems like /Factory/ would be the correct annotations here, that's not
* the case.
* As per Phil Thomson's advice on https://www.riverbankcomputing.com/pipermail/pyqt/2017-July/039450.html:
*
* "
* /Factory/ is used when the instance returned is guaranteed to be new to Python.
* In this case it isn't because it has already been seen when being returned by QgsProcessingAlgorithm::createInstance()
* (However for a different sub-class implemented in C++ then it would be the first time it was seen
* by Python so the /Factory/ on create() would be correct.)
*
* You might try using /TransferBack/ on create() instead - that might be the best compromise.
* "
*/

/**
* Creates a new instance of a layout item given the item metadata \a metadataId, target \a layout.
*/
QgsLayoutItem *createItem( int metadataId, QgsLayout *layout ) const SIP_FACTORY;
QgsLayoutItem *createItem( int metadataId, QgsLayout *layout ) const SIP_TRANSFERBACK;

/**
* Called when a newly created item of the associated metadata \a metadataId has been added to a layout.
Expand All @@ -390,10 +420,25 @@ class GUI_EXPORT QgsLayoutItemGuiRegistry : public QObject
*/
void newItemAddedToLayout( int metadataId, QgsLayoutItem *item );

/*
* IMPORTANT: While it seems like /Factory/ would be the correct annotations here, that's not
* the case.
* As per Phil Thomson's advice on https://www.riverbankcomputing.com/pipermail/pyqt/2017-July/039450.html:
*
* "
* /Factory/ is used when the instance returned is guaranteed to be new to Python.
* In this case it isn't because it has already been seen when being returned by QgsProcessingAlgorithm::createInstance()
* (However for a different sub-class implemented in C++ then it would be the first time it was seen
* by Python so the /Factory/ on create() would be correct.)
*
* You might try using /TransferBack/ on create() instead - that might be the best compromise.
* "
*/

/**
* Creates a new instance of a layout item configuration widget for the specified \a item.
*/
QgsLayoutItemBaseWidget *createItemWidget( QgsLayoutItem *item ) const SIP_FACTORY;
QgsLayoutItemBaseWidget *createItemWidget( QgsLayoutItem *item ) const SIP_TRANSFERBACK;

/**
* Creates a new rubber band item for the specified item \a metadataId and destination \a view.
Expand Down

0 comments on commit da59316

Please sign in to comment.