Skip to content

Commit

Permalink
Fix crash when using inverted polygons with heatmap renderer
Browse files Browse the repository at this point in the history 
Add methods to QgsRendererV2AbstractMetadata and QgsRendererV2Registry
to control renderer compatiblity by layer type. Should make it
easier to avoid this recurring bug popping up again in future.

Also add unit tests for QgsRendererV2Registry

Fix #14968
  • Loading branch information
@nyalldawson
nyalldawson committed Jun 7, 2016
1 parent fbc5e0f commit 94e1d5e
Show file tree
Hide file tree
Showing 8 changed files with 340 additions and 32 deletions.
57 changes: 48 additions & 9 deletions python/core/symbology-ng/qgsrendererv2registry.sip
View file
Expand Up @@ -13,6 +13,19 @@ class QgsRendererV2AbstractMetadata
%End

public:


//! Layer types the renderer is compatible with
//! @note added in QGIS 2.16
enum LayerType
{
PointLayer, //!< Compatible with point layers
LineLayer, //!< Compatible with line layers
PolygonLayer, //!< Compatible with polygon layers
All, //!< Compatible with all vector layers
};
typedef QFlags<QgsRendererV2AbstractMetadata::LayerType> LayerTypes;

QgsRendererV2AbstractMetadata( const QString& name, const QString& visibleName, const QIcon& icon = QIcon() );
virtual ~QgsRendererV2AbstractMetadata();

Expand All @@ -22,6 +35,11 @@ class QgsRendererV2AbstractMetadata
QIcon icon() const;
void setIcon( const QIcon& icon );

/** Returns flags indicating the types of layer the renderer is compatible with.
* @note added in QGIS 2.16
*/
virtual QgsRendererV2AbstractMetadata::LayerTypes compatibleLayerTypes() const;

/** Return new instance of the renderer given the DOM element. Returns NULL on error.
* Pure virtual function: must be implemented in derived classes. */
virtual QgsFeatureRendererV2* createRenderer( QDomElement& elem ) = 0 /Factory/;
Expand All @@ -37,6 +55,8 @@ class QgsRendererV2AbstractMetadata
virtual QgsFeatureRendererV2* createRendererFromSld( QDomElement& elem, QGis::GeometryType geomType ) /Factory/;
};

QFlags<QgsRendererV2AbstractMetadata::LayerType> operator|(QgsRendererV2AbstractMetadata::LayerType f1, QFlags<QgsRendererV2AbstractMetadata::LayerType> f2);

/**
Convenience metadata class that uses static functions to create renderer and its widget.
*/
Expand All @@ -55,15 +75,19 @@ class QgsRendererV2Metadata : QgsRendererV2AbstractMetadata
// QgsFeatureRendererV2* (*)( QDomElement&, QGis::GeometryType geomType ) createFromSldFunction() const;
// void setWidgetFunction( QgsRendererV2WidgetFunc f );

virtual QgsRendererV2AbstractMetadata::LayerTypes compatibleLayerTypes() const;

private:
QgsRendererV2Metadata(); // pretend this is private
};

/**
Registry of renderers.

This is a singleton, renderers can be added / removed at any time
/** \ingroup core
* \class QgsRendererV2Registry
* \brief Registry of renderers.
*
* This is a singleton, renderers can be added / removed at any time
*/

class QgsRendererV2Registry
{
%TypeHeaderCode
Expand All @@ -72,19 +96,33 @@ class QgsRendererV2Registry

public:

//! Returns a pointer to the QgsRendererV2Registry singleton
static QgsRendererV2Registry* instance();

//! add a renderer to registry. Takes ownership of the metadata object.
//! Adds a renderer to the registry. Takes ownership of the metadata object.
//! @param metadata renderer metadata
//! @returns true if renderer was added successfully, or false if renderer could not
//! be added (eg a renderer with a duplicate name already exists)
bool addRenderer( QgsRendererV2AbstractMetadata* metadata /Transfer/ );

//! remove renderer from registry
//! Removes a renderer from registry.
//! @param rendererName name of renderer to remove from registry
//! @returns true if renderer was sucessfully removed, or false if matching
//! renderer could not be found
bool removeRenderer( const QString& rendererName );

//! get metadata for particular renderer. Returns NULL if not found in registry.
//! Returns the metadata for a specified renderer. Returns NULL if a matching
//! renderer was not found in the registry.
QgsRendererV2AbstractMetadata* rendererMetadata( const QString& rendererName );

//! return a list of available renderers
QStringList renderersList();
//! Returns a list of available renderers.
//! @param layerTypes flags to filter the renderers by compatible layer types
QStringList renderersList( QgsRendererV2AbstractMetadata::LayerTypes layerTypes = QgsRendererV2AbstractMetadata::All ) const;

//! Returns a list of available renderers which are compatible with a specified layer.
//! @param layer vector layer
//! @note added in QGIS 2.16
QStringList renderersList( const QgsVectorLayer* layer ) const;

protected:
//! protected constructor
Expand All @@ -93,5 +131,6 @@ class QgsRendererV2Registry

private:
QgsRendererV2Registry( const QgsRendererV2Registry& rh );
//QgsRendererV2Registry& operator=( const QgsRendererV2Registry& rh );

};
57 changes: 51 additions & 6 deletions src/core/symbology-ng/qgsrendererv2registry.cpp
View file
Expand Up @@ -24,6 +24,7 @@
#include "qgsheatmaprenderer.h"
#include "qgs25drenderer.h"
#include "qgsnullsymbolrenderer.h"
#include "qgsvectorlayer.h"

QgsRendererV2Registry::QgsRendererV2Registry()
{
Expand Down Expand Up @@ -52,20 +53,32 @@ QgsRendererV2Registry::QgsRendererV2Registry()

addRenderer( new QgsRendererV2Metadata( "pointDisplacement",
QObject::tr( "Point displacement" ),
QgsPointDisplacementRenderer::create ) );
QgsPointDisplacementRenderer::create,
QIcon(),
nullptr,
QgsRendererV2AbstractMetadata::PointLayer ) );

addRenderer( new QgsRendererV2Metadata( "invertedPolygonRenderer",
QObject::tr( "Inverted polygons" ),
QgsInvertedPolygonRenderer::create ) );
QgsInvertedPolygonRenderer::create,
QIcon(),
nullptr,
QgsRendererV2AbstractMetadata::PolygonLayer ) );

addRenderer( new QgsRendererV2Metadata( "heatmapRenderer",
QObject::tr( "Heatmap" ),
QgsHeatmapRenderer::create ) );
QgsHeatmapRenderer::create,
QIcon(),
nullptr,
QgsRendererV2AbstractMetadata::PointLayer ) );


addRenderer( new QgsRendererV2Metadata( "25dRenderer",
QObject::tr( "2.5 D" ),
Qgs25DRenderer::create ) );
Qgs25DRenderer::create,
QIcon(),
nullptr,
QgsRendererV2AbstractMetadata::PolygonLayer ) );
}

QgsRendererV2Registry::~QgsRendererV2Registry()
Expand Down Expand Up @@ -108,7 +121,39 @@ QgsRendererV2AbstractMetadata* QgsRendererV2Registry::rendererMetadata( const QS

QgsRendererV2Metadata::~QgsRendererV2Metadata() {}

QStringList QgsRendererV2Registry::renderersList()
QStringList QgsRendererV2Registry::renderersList( QgsRendererV2AbstractMetadata::LayerTypes layerTypes ) const
{
return mRenderersOrder;
QStringList renderers;
Q_FOREACH ( const QString& renderer, mRenderersOrder )
{
if ( mRenderers.value( renderer )->compatibleLayerTypes() & layerTypes )
renderers << renderer;
}
return renderers;
}

QStringList QgsRendererV2Registry::renderersList( const QgsVectorLayer* layer ) const
{
QgsRendererV2AbstractMetadata::LayerType layerType = QgsRendererV2AbstractMetadata::All;

switch ( layer->geometryType() )
{
case QGis::Point:
layerType = QgsRendererV2AbstractMetadata::PointLayer;
break;

case QGis::Line:
layerType = QgsRendererV2AbstractMetadata::LineLayer;
break;

case QGis::Polygon:
layerType = QgsRendererV2AbstractMetadata::PolygonLayer;
break;

case QGis::UnknownGeometry:
case QGis::NoGeometry:
break;
}

return renderersList( layerType );
}
70 changes: 59 additions & 11 deletions src/core/symbology-ng/qgsrendererv2registry.h
View file
Expand Up @@ -36,6 +36,18 @@ class QgsRendererV2Widget;
class CORE_EXPORT QgsRendererV2AbstractMetadata
{
public:

//! Layer types the renderer is compatible with
//! @note added in QGIS 2.16
enum LayerType
{
PointLayer = 1, //!< Compatible with point layers
LineLayer = 2, //!< Compatible with line layers
PolygonLayer = 4, //!< Compatible with polygon layers
All = PointLayer | LineLayer | PolygonLayer, //!< Compatible with all vector layers
};
Q_DECLARE_FLAGS( LayerTypes, LayerType )

QgsRendererV2AbstractMetadata( const QString& name, const QString& visibleName, const QIcon& icon = QIcon() )
: mName( name )
, mVisibleName( visibleName )
Expand All @@ -49,6 +61,11 @@ class CORE_EXPORT QgsRendererV2AbstractMetadata
QIcon icon() const { return mIcon; }
void setIcon( const QIcon& icon ) { mIcon = icon; }

/** Returns flags indicating the types of layer the renderer is compatible with.
* @note added in QGIS 2.16
*/
virtual LayerTypes compatibleLayerTypes() const { return All; }

/** Return new instance of the renderer given the DOM element. Returns NULL on error.
* Pure virtual function: must be implemented in derived classes. */
virtual QgsFeatureRendererV2* createRenderer( QDomElement& elem ) = 0;
Expand All @@ -75,6 +92,9 @@ class CORE_EXPORT QgsRendererV2AbstractMetadata
};


Q_DECLARE_OPERATORS_FOR_FLAGS( QgsRendererV2AbstractMetadata::LayerTypes )


typedef QgsFeatureRendererV2*( *QgsRendererV2CreateFunc )( QDomElement& );
typedef QgsRendererV2Widget*( *QgsRendererV2WidgetFunc )( QgsVectorLayer*, QgsStyleV2*, QgsFeatureRendererV2* );
typedef QgsFeatureRendererV2*( *QgsRendererV2CreateFromSldFunc )( QDomElement&, QGis::GeometryType geomType );
Expand All @@ -92,11 +112,13 @@ class CORE_EXPORT QgsRendererV2Metadata : public QgsRendererV2AbstractMetadata
const QString& visibleName,
QgsRendererV2CreateFunc pfCreate,
const QIcon& icon = QIcon(),
QgsRendererV2WidgetFunc pfWidget = nullptr )
QgsRendererV2WidgetFunc pfWidget = nullptr,
QgsRendererV2AbstractMetadata::LayerTypes layerTypes = QgsRendererV2AbstractMetadata::All )
: QgsRendererV2AbstractMetadata( name, visibleName, icon )
, mCreateFunc( pfCreate )
, mWidgetFunc( pfWidget )
, mCreateFromSldFunc( nullptr )
, mLayerTypes( layerTypes )
{}

//! @note not available in python bindings
Expand All @@ -105,11 +127,13 @@ class CORE_EXPORT QgsRendererV2Metadata : public QgsRendererV2AbstractMetadata
QgsRendererV2CreateFunc pfCreate,
QgsRendererV2CreateFromSldFunc pfCreateFromSld,
const QIcon& icon = QIcon(),
QgsRendererV2WidgetFunc pfWidget = nullptr )
QgsRendererV2WidgetFunc pfWidget = nullptr,
QgsRendererV2AbstractMetadata::LayerTypes layerTypes = QgsRendererV2AbstractMetadata::All )
: QgsRendererV2AbstractMetadata( name, visibleName, icon )
, mCreateFunc( pfCreate )
, mWidgetFunc( pfWidget )
, mCreateFromSldFunc( pfCreateFromSld )
, mLayerTypes( layerTypes )
{}

virtual ~QgsRendererV2Metadata();
Expand All @@ -130,46 +154,70 @@ class CORE_EXPORT QgsRendererV2Metadata : public QgsRendererV2AbstractMetadata
//! @note not available in python bindings
void setWidgetFunction( QgsRendererV2WidgetFunc f ) { mWidgetFunc = f; }

virtual QgsRendererV2AbstractMetadata::LayerTypes compatibleLayerTypes() const override { return mLayerTypes; }

protected:
//! pointer to function that creates an instance of the renderer when loading project / style
QgsRendererV2CreateFunc mCreateFunc;
//! pointer to function that creates a widget for configuration of renderer's params
QgsRendererV2WidgetFunc mWidgetFunc;
//! pointer to function that creates an instance of the renderer from SLD
QgsRendererV2CreateFromSldFunc mCreateFromSldFunc;

private:

QgsRendererV2AbstractMetadata::LayerTypes mLayerTypes;
};

/**
Registry of renderers.

This is a singleton, renderers can be added / removed at any time
/** \ingroup core
* \class QgsRendererV2Registry
* \brief Registry of renderers.
*
* This is a singleton, renderers can be added / removed at any time
*/

class CORE_EXPORT QgsRendererV2Registry
{
public:

//! Returns a pointer to the QgsRendererV2Registry singleton
static QgsRendererV2Registry* instance();

//! add a renderer to registry. Takes ownership of the metadata object.
//! Adds a renderer to the registry. Takes ownership of the metadata object.
//! @param metadata renderer metadata
//! @returns true if renderer was added successfully, or false if renderer could not
//! be added (eg a renderer with a duplicate name already exists)
bool addRenderer( QgsRendererV2AbstractMetadata* metadata );

//! remove renderer from registry
//! Removes a renderer from registry.
//! @param rendererName name of renderer to remove from registry
//! @returns true if renderer was sucessfully removed, or false if matching
//! renderer could not be found
bool removeRenderer( const QString& rendererName );

//! get metadata for particular renderer. Returns NULL if not found in registry.
//! Returns the metadata for a specified renderer. Returns NULL if a matching
//! renderer was not found in the registry.
QgsRendererV2AbstractMetadata* rendererMetadata( const QString& rendererName );

//! return a list of available renderers
QStringList renderersList();
//! Returns a list of available renderers.
//! @param layerTypes flags to filter the renderers by compatible layer types
QStringList renderersList( QgsRendererV2AbstractMetadata::LayerTypes layerTypes = QgsRendererV2AbstractMetadata::All ) const;

//! Returns a list of available renderers which are compatible with a specified layer.
//! @param layer vector layer
//! @note added in QGIS 2.16
QStringList renderersList( const QgsVectorLayer* layer ) const;

protected:
//! protected constructor
QgsRendererV2Registry();
~QgsRendererV2Registry();

//! Map of name to renderer
QMap<QString, QgsRendererV2AbstractMetadata*> mRenderers;

//! list to keep order in which renderers have been added
//! List of renderers, maintained in the order that they have been added
QStringList mRenderersOrder;

private:
Expand Down
5 changes: 2 additions & 3 deletions src/gui/symbology-ng/qgsinvertedpolygonrendererwidget.cpp
View file
Expand Up @@ -69,14 +69,13 @@ QgsInvertedPolygonRendererWidget::QgsInvertedPolygonRendererWidget( QgsVectorLay

int currentEmbeddedIdx = 0;
//insert possible renderer types
QStringList rendererList = QgsRendererV2Registry::instance()->renderersList();
QStringList rendererList = QgsRendererV2Registry::instance()->renderersList( QgsRendererV2AbstractMetadata::PolygonLayer );
QStringList::const_iterator it = rendererList.constBegin();
int idx = 0;
mRendererComboBox->blockSignals( true );
for ( ; it != rendererList.constEnd(); ++it, ++idx )
{
if (( *it != "invertedPolygonRenderer" ) && //< an inverted renderer cannot contain another inverted renderer
( *it != "pointDisplacement" ) ) //< an inverted renderer can only contain a polygon renderer
if ( *it != "invertedPolygonRenderer" ) //< an inverted renderer cannot contain another inverted renderer
{
QgsRendererV2AbstractMetadata* m = QgsRendererV2Registry::instance()->rendererMetadata( *it );
mRendererComboBox->addItem( m->icon(), m->visibleName(), /* data */ *it );
Expand Down
4 changes: 2 additions & 2 deletions src/gui/symbology-ng/qgspointdisplacementrendererwidget.cpp
View file
Expand Up @@ -87,11 +87,11 @@ QgsPointDisplacementRendererWidget::QgsPointDisplacementRendererWidget( QgsVecto
}

//insert possible renderer types
QStringList rendererList = QgsRendererV2Registry::instance()->renderersList();
QStringList rendererList = QgsRendererV2Registry::instance()->renderersList( QgsRendererV2AbstractMetadata::PointLayer );
QStringList::const_iterator it = rendererList.constBegin();
for ( ; it != rendererList.constEnd(); ++it )
{
if ( *it != "pointDisplacement" && *it != "heatmapRenderer" && *it != "invertedPolygonRenderer" )
if ( *it != "pointDisplacement" )
{
QgsRendererV2AbstractMetadata* m = QgsRendererV2Registry::instance()->rendererMetadata( *it );
mRendererComboBox->addItem( m->icon(), m->visibleName(), *it );
Expand Down
2 changes: 1 addition & 1 deletion src/gui/symbology-ng/qgsrendererv2propertiesdialog.cpp
View file
Expand Up @@ -100,7 +100,7 @@ QgsRendererV2PropertiesDialog::QgsRendererV2PropertiesDialog( QgsVectorLayer* la
_initRendererWidgetFunctions();

QgsRendererV2Registry* reg = QgsRendererV2Registry::instance();
QStringList renderers = reg->renderersList();
QStringList renderers = reg->renderersList( mLayer );
Q_FOREACH ( const QString& name, renderers )
{
QgsRendererV2AbstractMetadata* m = reg->rendererMetadata( name );
Expand Down
1 change: 1 addition & 0 deletions tests/src/python/CMakeLists.txt
View file
Expand Up @@ -68,6 +68,7 @@ ADD_PYTHON_TEST(PyQgsRasterLayer test_qgsrasterlayer.py)
ADD_PYTHON_TEST(PyQgsRectangle test_qgsrectangle.py)
ADD_PYTHON_TEST(PyQgsRelation test_qgsrelation.py)
ADD_PYTHON_TEST(PyQgsRelationManager test_qgsrelationmanager.py)
ADD_PYTHON_TEST(PyQgsRendererV2 test_qgsrendererv2.py)
ADD_PYTHON_TEST(PyQgsRulebasedRenderer test_qgsrulebasedrenderer.py)
ADD_PYTHON_TEST(PyQgsSingleSymbolRenderer test_qgssinglesymbolrenderer.py)
ADD_PYTHON_TEST(PyQgsShapefileProvider test_provider_shapefile.py)
Expand Down

0 comments on commit 94e1d5e

Please sign in to comment.