From 49e9b613b88ebe92b282fbde14263c74e7fd7b7f Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Wed, 16 Sep 2020 15:31:42 +1000 Subject: [PATCH] [layouts] Keep a separate flag for whether only a subset of layers are to be clipped from the project, instead of just tracking this by the presence of any checked layers Avoids inconsistencies between the layers which are visibly clipped on the map vs the options which are set in the GUI. --- .../layout/qgslayoutitemmap.sip.in | 30 +++++++++++++++-- .../qgsmapclippingregion.sip.in | 33 +++++++++++++++++-- src/core/layout/qgslayoutitemmap.cpp | 17 ++++++++++ src/core/layout/qgslayoutitemmap.h | 23 +++++++++++-- src/core/qgsmapclippingregion.cpp | 15 ++++++++- src/core/qgsmapclippingregion.h | 24 +++++++++++++- src/gui/layout/qgslayoutmapwidget.cpp | 8 ++--- .../test_qgslayoutatlasclippingsettings.py | 10 ++++++ tests/src/python/test_qgsmapclippingregion.py | 14 ++++++++ tests/src/python/test_qgsmapclippingutils.py | 1 + 10 files changed, 163 insertions(+), 12 deletions(-) diff --git a/python/core/auto_generated/layout/qgslayoutitemmap.sip.in b/python/core/auto_generated/layout/qgslayoutitemmap.sip.in index 9ef40606b54..5a4798174c0 100644 --- a/python/core/auto_generated/layout/qgslayoutitemmap.sip.in +++ b/python/core/auto_generated/layout/qgslayoutitemmap.sip.in @@ -67,13 +67,35 @@ Returns ``True`` if labels should only be placed inside the atlas feature geomet Sets whether labels should only be placed inside the atlas feature geometry. .. seealso:: :py:func:`forceLabelsInsideFeature` +%End + + bool restrictToLayers() const; +%Docstring +Returns ``True`` if clipping should be restricted to a subset of layers. + +.. seealso:: :py:func:`layersToClip` + +.. seealso:: :py:func:`setRestrictToLayers` +%End + + void setRestrictToLayers( bool enabled ); +%Docstring +Sets whether clipping should be restricted to a subset of layers. + +.. seealso:: :py:func:`setLayersToClip` + +.. seealso:: :py:func:`restrictToLayers` %End QList< QgsMapLayer * > layersToClip() const; %Docstring Returns the list of map layers to clip to the atlas feature. -If the returned list is empty then all layers will be clipped. +.. note:: + + This setting is only used if :py:func:`~QgsLayoutItemMapAtlasClippingSettings.restrictToLayers` is ``True``. + +.. seealso:: :py:func:`restrictedLayers` .. seealso:: :py:func:`setLayersToClip` %End @@ -82,7 +104,11 @@ If the returned list is empty then all layers will be clipped. %Docstring Sets the list of map ``layers`` to clip to the atlas feature. -If the ``layers`` list is empty then all layers will be clipped. +.. note:: + + This setting is only used if :py:func:`~QgsLayoutItemMapAtlasClippingSettings.restrictToLayers` is ``True``. + +.. seealso:: :py:func:`restrictedLayers` .. seealso:: :py:func:`layersToClip` %End diff --git a/python/core/auto_generated/qgsmapclippingregion.sip.in b/python/core/auto_generated/qgsmapclippingregion.sip.in index c5493f6fa2f..34aeb2b68d8 100644 --- a/python/core/auto_generated/qgsmapclippingregion.sip.in +++ b/python/core/auto_generated/qgsmapclippingregion.sip.in @@ -64,6 +64,24 @@ Sets the feature clipping ``type``. This setting is only used while rendering vector layers, for other layer types it is ignored. .. seealso:: :py:func:`featureClip` +%End + + bool restrictToLayers() const; +%Docstring +Returns ``True`` if clipping should be restricted to a subset of layers. + +.. seealso:: :py:func:`restrictedLayers` + +.. seealso:: :py:func:`setRestrictToLayers` +%End + + void setRestrictToLayers( bool enabled ); +%Docstring +Sets whether clipping should be restricted to a subset of layers. + +.. seealso:: :py:func:`setRestrictedLayers` + +.. seealso:: :py:func:`restrictToLayers` %End void setRestrictedLayers( const QList< QgsMapLayer * > &layers ); @@ -72,9 +90,14 @@ Sets a list of ``layers`` to restrict the clipping region effects to. By default the clipping region applies to all layers. -.. seealso:: :py:func:`restrictedLayers` -%End +.. note:: + This setting is only used if :py:func:`~QgsMapClippingRegion.restrictToLayers` is ``True``. + +.. seealso:: :py:func:`restrictedLayers` + +.. seealso:: :py:func:`setRestrictToLayers` +%End QList< QgsMapLayer * > restrictedLayers() const; %Docstring @@ -82,7 +105,13 @@ Returns the list of layers to restrict the clipping region effects to. If the list is empty then the clipping will be applied to all layers. +.. note:: + + This setting is only used if :py:func:`~QgsMapClippingRegion.restrictToLayers` is ``True``. + .. seealso:: :py:func:`setRestrictedLayers` + +.. seealso:: :py:func:`restrictToLayers` %End bool appliesToLayer( const QgsMapLayer *layer ) const; diff --git a/src/core/layout/qgslayoutitemmap.cpp b/src/core/layout/qgslayoutitemmap.cpp index fdc93bccf4f..db553adc6fb 100644 --- a/src/core/layout/qgslayoutitemmap.cpp +++ b/src/core/layout/qgslayoutitemmap.cpp @@ -1554,6 +1554,7 @@ QgsMapSettings QgsLayoutItemMap::mapSettings( const QgsRectangle &extent, QSizeF QgsMapClippingRegion region( clipGeom ); region.setFeatureClip( mAtlasClippingSettings->featureClippingType() ); region.setRestrictedLayers( mAtlasClippingSettings->layersToClip() ); + region.setRestrictToLayers( mAtlasClippingSettings->restrictToLayers() ); jobMapSettings.addClippingRegion( region ); if ( mAtlasClippingSettings->forceLabelsInsideFeature() ) @@ -2784,6 +2785,20 @@ void QgsLayoutItemMapAtlasClippingSettings::setForceLabelsInsideFeature( bool fo emit changed(); } +bool QgsLayoutItemMapAtlasClippingSettings::restrictToLayers() const +{ + return mRestrictToLayers; +} + +void QgsLayoutItemMapAtlasClippingSettings::setRestrictToLayers( bool enabled ) +{ + if ( mRestrictToLayers == enabled ) + return; + + mRestrictToLayers = enabled; + emit changed(); +} + QList QgsLayoutItemMapAtlasClippingSettings::layersToClip() const { return _qgis_listRefToRaw( mLayersToClip ); @@ -2801,6 +2816,7 @@ bool QgsLayoutItemMapAtlasClippingSettings::writeXml( QDomElement &element, QDom settingsElem.setAttribute( QStringLiteral( "enabled" ), mClipToAtlasFeature ? QStringLiteral( "1" ) : QStringLiteral( "0" ) ); settingsElem.setAttribute( QStringLiteral( "forceLabelsInside" ), mForceLabelsInsideFeature ? QStringLiteral( "1" ) : QStringLiteral( "0" ) ); settingsElem.setAttribute( QStringLiteral( "clippingType" ), QString::number( static_cast( mFeatureClippingType ) ) ); + settingsElem.setAttribute( QStringLiteral( "restrictLayers" ), mRestrictToLayers ? QStringLiteral( "1" ) : QStringLiteral( "0" ) ); //layer set QDomElement layerSetElem = document.createElement( QStringLiteral( "layersToClip" ) ); @@ -2831,6 +2847,7 @@ bool QgsLayoutItemMapAtlasClippingSettings::readXml( const QDomElement &element, mClipToAtlasFeature = settingsElem.attribute( QStringLiteral( "enabled" ), QStringLiteral( "0" ) ).toInt(); mForceLabelsInsideFeature = settingsElem.attribute( QStringLiteral( "forceLabelsInside" ), QStringLiteral( "0" ) ).toInt(); mFeatureClippingType = static_cast< QgsMapClippingRegion::FeatureClippingType >( settingsElem.attribute( QStringLiteral( "clippingType" ), QStringLiteral( "0" ) ).toInt() ); + mRestrictToLayers = settingsElem.attribute( QStringLiteral( "restrictLayers" ), QStringLiteral( "0" ) ).toInt(); mLayersToClip.clear(); QDomNodeList layerSetNodeList = settingsElem.elementsByTagName( QStringLiteral( "layersToClip" ) ); diff --git a/src/core/layout/qgslayoutitemmap.h b/src/core/layout/qgslayoutitemmap.h index 54e3cb62ca7..dee007d6ec1 100644 --- a/src/core/layout/qgslayoutitemmap.h +++ b/src/core/layout/qgslayoutitemmap.h @@ -89,11 +89,28 @@ class CORE_EXPORT QgsLayoutItemMapAtlasClippingSettings : public QObject */ void setForceLabelsInsideFeature( bool forceInside ); + /** + * Returns TRUE if clipping should be restricted to a subset of layers. + * + * \see layersToClip() + * \see setRestrictToLayers() + */ + bool restrictToLayers() const; + + /** + * Sets whether clipping should be restricted to a subset of layers. + * + * \see setLayersToClip() + * \see restrictToLayers() + */ + void setRestrictToLayers( bool enabled ); + /** * Returns the list of map layers to clip to the atlas feature. * - * If the returned list is empty then all layers will be clipped. + * \note This setting is only used if restrictToLayers() is TRUE. * + * \see restrictedLayers() * \see setLayersToClip() */ QList< QgsMapLayer * > layersToClip() const; @@ -101,8 +118,9 @@ class CORE_EXPORT QgsLayoutItemMapAtlasClippingSettings : public QObject /** * Sets the list of map \a layers to clip to the atlas feature. * - * If the \a layers list is empty then all layers will be clipped. + * \note This setting is only used if restrictToLayers() is TRUE. * + * \see restrictedLayers() * \see layersToClip() */ void setLayersToClip( const QList< QgsMapLayer * > &layers ); @@ -135,6 +153,7 @@ class CORE_EXPORT QgsLayoutItemMapAtlasClippingSettings : public QObject QgsLayoutItemMap *mMap = nullptr; bool mClipToAtlasFeature = false; + bool mRestrictToLayers = false; QList< QgsMapLayerRef > mLayersToClip; QgsMapClippingRegion::FeatureClippingType mFeatureClippingType = QgsMapClippingRegion::FeatureClippingType::ClipPainterOnly; bool mForceLabelsInsideFeature = false; diff --git a/src/core/qgsmapclippingregion.cpp b/src/core/qgsmapclippingregion.cpp index 91ebb45aef4..cc7afd73673 100644 --- a/src/core/qgsmapclippingregion.cpp +++ b/src/core/qgsmapclippingregion.cpp @@ -39,9 +39,12 @@ QList QgsMapClippingRegion::restrictedLayers() const bool QgsMapClippingRegion::appliesToLayer( const QgsMapLayer *layer ) const { - if ( mRestrictToLayersList.empty() ) + if ( !mRestrictToLayers ) return true; + if ( mRestrictToLayersList.empty() ) + return false; + auto it = std::find_if( mRestrictToLayersList.begin(), mRestrictToLayersList.end(), [layer]( const QgsWeakMapLayerPointer & item ) -> bool { return item == layer; @@ -49,4 +52,14 @@ bool QgsMapClippingRegion::appliesToLayer( const QgsMapLayer *layer ) const return it != mRestrictToLayersList.end(); } +bool QgsMapClippingRegion::restrictToLayers() const +{ + return mRestrictToLayers; +} + +void QgsMapClippingRegion::setRestrictToLayers( bool enabled ) +{ + mRestrictToLayers = enabled; +} + diff --git a/src/core/qgsmapclippingregion.h b/src/core/qgsmapclippingregion.h index ae39c5d3a48..75e41bc42fc 100644 --- a/src/core/qgsmapclippingregion.h +++ b/src/core/qgsmapclippingregion.h @@ -88,22 +88,43 @@ class CORE_EXPORT QgsMapClippingRegion mFeatureClip = type; } + /** + * Returns TRUE if clipping should be restricted to a subset of layers. + * + * \see restrictedLayers() + * \see setRestrictToLayers() + */ + bool restrictToLayers() const; + + /** + * Sets whether clipping should be restricted to a subset of layers. + * + * \see setRestrictedLayers() + * \see restrictToLayers() + */ + void setRestrictToLayers( bool enabled ); + /** * Sets a list of \a layers to restrict the clipping region effects to. * * By default the clipping region applies to all layers. * + * \note This setting is only used if restrictToLayers() is TRUE. + * * \see restrictedLayers() + * \see setRestrictToLayers() */ void setRestrictedLayers( const QList< QgsMapLayer * > &layers ); - /** * Returns the list of layers to restrict the clipping region effects to. * * If the list is empty then the clipping will be applied to all layers. * + * \note This setting is only used if restrictToLayers() is TRUE. + * * \see setRestrictedLayers() + * \see restrictToLayers() */ QList< QgsMapLayer * > restrictedLayers() const; @@ -117,6 +138,7 @@ class CORE_EXPORT QgsMapClippingRegion //! Geometry of clipping region (in destination map coordinates and CRS) QgsGeometry mGeometry; + bool mRestrictToLayers = false; QgsWeakMapLayerPointerList mRestrictToLayersList; FeatureClippingType mFeatureClip = FeatureClippingType::ClipToIntersection; diff --git a/src/gui/layout/qgslayoutmapwidget.cpp b/src/gui/layout/qgslayoutmapwidget.cpp index 338700ef8ac..a76b11a59d7 100644 --- a/src/gui/layout/qgslayoutmapwidget.cpp +++ b/src/gui/layout/qgslayoutmapwidget.cpp @@ -2016,7 +2016,7 @@ QgsLayoutMapClippingWidget::QgsLayoutMapClippingWidget( QgsLayoutItemMap *map ) { mBlockUpdates = true; mMapItem->beginCommand( tr( "Change Atlas Clipping Layers" ) ); - mMapItem->atlasClippingSettings()->setLayersToClip( mLayerModel->layersChecked( ) ); + mMapItem->atlasClippingSettings()->setRestrictToLayers( true ); mMapItem->endCommand(); mBlockUpdates = false; } @@ -2027,7 +2027,7 @@ QgsLayoutMapClippingWidget::QgsLayoutMapClippingWidget( QgsLayoutItemMap *map ) { mBlockUpdates = true; mMapItem->beginCommand( tr( "Change Atlas Clipping Layers" ) ); - mMapItem->atlasClippingSettings()->setLayersToClip( QList< QgsMapLayer * >() ); + mMapItem->atlasClippingSettings()->setRestrictToLayers( false ); mMapItem->endCommand(); mBlockUpdates = false; } @@ -2136,8 +2136,8 @@ void QgsLayoutMapClippingWidget::updateGuiElements() mAtlasClippingTypeComboBox->setCurrentIndex( mAtlasClippingTypeComboBox->findData( static_cast< int >( mMapItem->atlasClippingSettings()->featureClippingType() ) ) ); mForceLabelsInsideCheckBox->setChecked( mMapItem->atlasClippingSettings()->forceLabelsInsideFeature() ); - mRadioClipAllLayers->setChecked( mMapItem->atlasClippingSettings()->layersToClip().isEmpty() ); - mRadioClipSelectedLayers->setChecked( !mMapItem->atlasClippingSettings()->layersToClip().isEmpty() ); + mRadioClipAllLayers->setChecked( !mMapItem->atlasClippingSettings()->restrictToLayers() ); + mRadioClipSelectedLayers->setChecked( mMapItem->atlasClippingSettings()->restrictToLayers() ); mLayerModel->setLayersChecked( mMapItem->atlasClippingSettings()->layersToClip() ); mClipToItemCheckBox->setChecked( mMapItem->itemClippingSettings()->enabled() ); diff --git a/tests/src/python/test_qgslayoutatlasclippingsettings.py b/tests/src/python/test_qgslayoutatlasclippingsettings.py index 2d04d57f41a..15dcbd5fe5c 100644 --- a/tests/src/python/test_qgslayoutatlasclippingsettings.py +++ b/tests/src/python/test_qgslayoutatlasclippingsettings.py @@ -74,6 +74,14 @@ class TestQgsLayoutItemMapAtlasClippingSettings(unittest.TestCase): p.removeMapLayer(l1.id()) self.assertCountEqual(settings.layersToClip(), [l2]) + settings.setRestrictToLayers(False) + self.assertFalse(settings.restrictToLayers()) + self.assertEqual(len(spy), 4) + + settings.setRestrictToLayers(True) + self.assertTrue(settings.restrictToLayers()) + self.assertEqual(len(spy), 5) + def testSaveRestore(self): p = QgsProject() l1 = QgsVectorLayer("Point?field=fldtxt:string&field=fldint:integer", @@ -89,6 +97,7 @@ class TestQgsLayoutItemMapAtlasClippingSettings(unittest.TestCase): settings.setEnabled(True) settings.setFeatureClippingType(QgsMapClippingRegion.FeatureClippingType.NoClipping) settings.setForceLabelsInsideFeature(True) + settings.setRestrictToLayers(True) settings.setLayersToClip([l2]) # save map to xml @@ -107,6 +116,7 @@ class TestQgsLayoutItemMapAtlasClippingSettings(unittest.TestCase): self.assertEqual(map2.atlasClippingSettings().featureClippingType(), QgsMapClippingRegion.FeatureClippingType.NoClipping) self.assertTrue(map2.atlasClippingSettings().forceLabelsInsideFeature()) self.assertEqual(map2.atlasClippingSettings().layersToClip(), [l2]) + self.assertTrue(map2.atlasClippingSettings().restrictToLayers()) if __name__ == '__main__': diff --git a/tests/src/python/test_qgsmapclippingregion.py b/tests/src/python/test_qgsmapclippingregion.py index 68796021a0a..9092032fef6 100644 --- a/tests/src/python/test_qgsmapclippingregion.py +++ b/tests/src/python/test_qgsmapclippingregion.py @@ -40,6 +40,10 @@ class TestQgsMapClippingRegion(unittest.TestCase): self.assertEqual(len(region.restrictedLayers()), 0) region.setRestrictedLayers([layer, layer2]) self.assertCountEqual(region.restrictedLayers(), [layer, layer2]) + region.setRestrictToLayers(False) + self.assertFalse(region.restrictToLayers()) + region.setRestrictToLayers(True) + self.assertTrue(region.restrictToLayers()) def testAppliesToLayer(self): layer = QgsVectorLayer("Point?field=fldtxt:string&field=fldint:integer", @@ -58,6 +62,16 @@ class TestQgsMapClippingRegion(unittest.TestCase): region.setRestrictedLayers([layer, layer2]) self.assertTrue(region.appliesToLayer(layer)) self.assertTrue(region.appliesToLayer(layer2)) + self.assertTrue(region.appliesToLayer(layer3)) + + region.setRestrictToLayers(True) + self.assertTrue(region.appliesToLayer(layer)) + self.assertTrue(region.appliesToLayer(layer2)) + self.assertFalse(region.appliesToLayer(layer3)) + + region.setRestrictedLayers([]) + self.assertFalse(region.appliesToLayer(layer)) + self.assertFalse(region.appliesToLayer(layer2)) self.assertFalse(region.appliesToLayer(layer3)) diff --git a/tests/src/python/test_qgsmapclippingutils.py b/tests/src/python/test_qgsmapclippingutils.py index a10e84b49ae..d330fa95f7b 100644 --- a/tests/src/python/test_qgsmapclippingutils.py +++ b/tests/src/python/test_qgsmapclippingutils.py @@ -43,6 +43,7 @@ class TestQgsMapClippingUtils(unittest.TestCase): region = QgsMapClippingRegion(QgsGeometry.fromWkt('Polygon((0 0, 1 0, 1 1, 0 1, 0 0))')) region2 = QgsMapClippingRegion(QgsGeometry.fromWkt('Polygon((0 0, 0.1 0, 0.1 2, 0 2, 0 0))')) region2.setRestrictedLayers([layer]) + region2.setRestrictToLayers(True) ms = QgsMapSettings() ms.addClippingRegion(region) ms.addClippingRegion(region2)