Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Masking] Fix copy/paste of symbol or converting renderer #60325

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions python/PyQt6/core/auto_additions/qgssymbollayerutils.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@
QgsSymbolLayerUtils.tileSize = staticmethod(QgsSymbolLayerUtils.tileSize)
QgsSymbolLayerUtils.clearSymbolLayerIds = staticmethod(QgsSymbolLayerUtils.clearSymbolLayerIds)
QgsSymbolLayerUtils.resetSymbolLayerIds = staticmethod(QgsSymbolLayerUtils.resetSymbolLayerIds)
QgsSymbolLayerUtils.clearSymbolLayerMasks = staticmethod(QgsSymbolLayerUtils.clearSymbolLayerMasks)
QgsSymbolLayerUtils.collectSymbolLayerClipGeometries = staticmethod(QgsSymbolLayerUtils.collectSymbolLayerClipGeometries)
QgsSymbolLayerUtils.__group__ = ['symbology']
except (NameError, AttributeError):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@ Returns a list of references to symbol layers that are masked by the sub symbol'
.. seealso:: :py:func:`setMasks`
%End

virtual void clearMasks();


void setMasks( const QList<QgsSymbolLayerReference> &maskedLayers );
%Docstring
Sets the symbol layers that will be masked by the sub symbol's shape.
Expand All @@ -100,8 +103,6 @@ Sets the symbol layers that will be masked by the sub symbol's shape.
QgsMaskMarkerSymbolLayer( const QgsMaskMarkerSymbolLayer & );
};



/************************************************************************
* This file has been generated automatically from *
* *
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -639,6 +639,15 @@ Returns masks defined by this symbol layer.
This is a list of symbol layers of other layers that should be occluded.

.. versionadded:: 3.12
%End

virtual void clearMasks();
%Docstring
Remove masks defined by this symbol layer.

.. seealso:: :py:func:`masks`

.. versionadded:: 3.42
%End

virtual void prepareMasks( const QgsSymbolRenderContext &context );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1020,6 +1020,13 @@ Regenerate recursively unique id from all ``symbol`` symbol layers
Regenerate recursively unique id from ``symbolLayer`` and its children

.. versionadded:: 3.30
%End

static void clearSymbolLayerMasks( QgsSymbol *symbol );
%Docstring
Remove recursively masks from all ``symbol`` symbol layers

.. versionadded:: 3.42
%End

static QVector< QgsGeometry > collectSymbolLayerClipGeometries( const QgsRenderContext &context, const QString &symbolLayerId, const QRectF &bounds );
Expand Down
1 change: 1 addition & 0 deletions python/core/auto_additions/qgssymbollayerutils.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@
QgsSymbolLayerUtils.tileSize = staticmethod(QgsSymbolLayerUtils.tileSize)
QgsSymbolLayerUtils.clearSymbolLayerIds = staticmethod(QgsSymbolLayerUtils.clearSymbolLayerIds)
QgsSymbolLayerUtils.resetSymbolLayerIds = staticmethod(QgsSymbolLayerUtils.resetSymbolLayerIds)
QgsSymbolLayerUtils.clearSymbolLayerMasks = staticmethod(QgsSymbolLayerUtils.clearSymbolLayerMasks)
QgsSymbolLayerUtils.collectSymbolLayerClipGeometries = staticmethod(QgsSymbolLayerUtils.collectSymbolLayerClipGeometries)
QgsSymbolLayerUtils.__group__ = ['symbology']
except (NameError, AttributeError):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@ Returns a list of references to symbol layers that are masked by the sub symbol'
.. seealso:: :py:func:`setMasks`
%End

virtual void clearMasks();


void setMasks( const QList<QgsSymbolLayerReference> &maskedLayers );
%Docstring
Sets the symbol layers that will be masked by the sub symbol's shape.
Expand All @@ -100,8 +103,6 @@ Sets the symbol layers that will be masked by the sub symbol's shape.
QgsMaskMarkerSymbolLayer( const QgsMaskMarkerSymbolLayer & );
};



/************************************************************************
* This file has been generated automatically from *
* *
Expand Down
9 changes: 9 additions & 0 deletions python/core/auto_generated/symbology/qgssymbollayer.sip.in
Original file line number Diff line number Diff line change
Expand Up @@ -639,6 +639,15 @@ Returns masks defined by this symbol layer.
This is a list of symbol layers of other layers that should be occluded.

.. versionadded:: 3.12
%End

virtual void clearMasks();
%Docstring
Remove masks defined by this symbol layer.

.. seealso:: :py:func:`masks`

.. versionadded:: 3.42
%End

virtual void prepareMasks( const QgsSymbolRenderContext &context );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1020,6 +1020,13 @@ Regenerate recursively unique id from all ``symbol`` symbol layers
Regenerate recursively unique id from ``symbolLayer`` and its children

.. versionadded:: 3.30
%End

static void clearSymbolLayerMasks( QgsSymbol *symbol );
%Docstring
Remove recursively masks from all ``symbol`` symbol layers

.. versionadded:: 3.42
%End

static QVector< QgsGeometry > collectSymbolLayerClipGeometries( const QgsRenderContext &context, const QString &symbolLayerId, const QRectF &bounds );
Expand Down
4 changes: 3 additions & 1 deletion src/app/qgsapplayertreeviewmenuprovider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,10 @@
#include "qgsmapcanvasutils.h"
#include "qgsmaplayeraction.h"
#include "qgsvectortilelayer.h"
#include "qgsvectortiledataprovider.h"
#include "qgsproviderregistry.h"
#include "qgsprovidermetadata.h"
#include "qgsrasterlabeling.h"
#include "qgssymbollayerutils.h"

QgsAppLayerTreeViewMenuProvider::QgsAppLayerTreeViewMenuProvider( QgsLayerTreeView *view, QgsMapCanvas *canvas )
: mView( view )
Expand Down Expand Up @@ -1185,6 +1185,7 @@ void QgsAppLayerTreeViewMenuProvider::pasteVectorSymbol( const QString &layerId
originalSymbol = embeddedRenderer->symbol();
}
std::unique_ptr<QgsSymbol> tempSymbol( QgsSymbolLayerUtils::symbolFromMimeData( QApplication::clipboard()->mimeData() ) );
QgsSymbolLayerUtils::resetSymbolLayerIds( tempSymbol.get() );
if ( !tempSymbol )
return;

Expand Down Expand Up @@ -1312,6 +1313,7 @@ void QgsAppLayerTreeViewMenuProvider::pasteSymbolLegendNodeSymbol( const QString
QgsVectorLayer *vlayer = qobject_cast<QgsVectorLayer *>( node->layerNode()->layer() );

std::unique_ptr<QgsSymbol> tempSymbol( QgsSymbolLayerUtils::symbolFromMimeData( QApplication::clipboard()->mimeData() ) );
QgsSymbolLayerUtils::resetSymbolLayerIds( tempSymbol.get() );
if ( tempSymbol && tempSymbol->type() == originalSymbol->type() )
{
node->setSymbol( tempSymbol.release() );
Expand Down
7 changes: 6 additions & 1 deletion src/core/symbology/qgscategorizedsymbolrenderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1445,7 +1445,10 @@ QgsCategorizedSymbolRenderer *QgsCategorizedSymbolRenderer::convertFromRenderer(
QgsSymbolList symbols = const_cast<QgsFeatureRenderer *>( renderer )->symbols( context );
if ( !symbols.isEmpty() )
{
r->setSourceSymbol( symbols.at( 0 )->clone() );
QgsSymbol *newSymbol = symbols.at( 0 )->clone();
QgsSymbolLayerUtils::resetSymbolLayerIds( newSymbol );
QgsSymbolLayerUtils::clearSymbolLayerMasks( newSymbol );
r->setSourceSymbol( newSymbol );
}
}

Expand Down Expand Up @@ -1539,6 +1542,7 @@ QgsCategoryList QgsCategorizedSymbolRenderer::createCategories( const QList<QVar
for ( const QVariant &value : vals )
{
QgsSymbol *newSymbol = symbol->clone();
QgsSymbolLayerUtils::resetSymbolLayerIds( newSymbol );
if ( !QgsVariantUtils::isNull( value ) )
{
const int fieldIdx = fields.lookupField( attributeName );
Expand All @@ -1557,6 +1561,7 @@ QgsCategoryList QgsCategorizedSymbolRenderer::createCategories( const QList<QVar

// add null (default) value
QgsSymbol *newSymbol = symbol->clone();
QgsSymbolLayerUtils::resetSymbolLayerIds( newSymbol );
cats.append( QgsRendererCategory( QVariant(), newSymbol, QString(), true ) );

return cats;
Expand Down
5 changes: 5 additions & 0 deletions src/core/symbology/qgsmasksymbollayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,11 @@ void QgsMaskMarkerSymbolLayer::setMasks( const QList<QgsSymbolLayerReference> &m
mMaskedSymbolLayers = maskedLayers;
}

void QgsMaskMarkerSymbolLayer::clearMasks()
{
mMaskedSymbolLayers.clear();
}

QRectF QgsMaskMarkerSymbolLayer::bounds( QPointF point, QgsSymbolRenderContext &context )
{
return mSymbol->bounds( point, context.renderContext() );
Expand Down
4 changes: 2 additions & 2 deletions src/core/symbology/qgsmasksymbollayer.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ class CORE_EXPORT QgsMaskMarkerSymbolLayer : public QgsMarkerSymbolLayer
*/
QList<QgsSymbolLayerReference> masks() const override;

void clearMasks() override;

/**
* Sets the symbol layers that will be masked by the sub symbol's shape.
* \param maskedLayers list of references to symbol layers
Expand All @@ -101,5 +103,3 @@ class CORE_EXPORT QgsMaskMarkerSymbolLayer : public QgsMarkerSymbolLayer
};

#endif


4 changes: 4 additions & 0 deletions src/core/symbology/qgssymbollayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -953,6 +953,10 @@ QList<QgsSymbolLayerReference> QgsSymbolLayer::masks() const
return {};
}

void QgsSymbolLayer::clearMasks()
{
}

double QgsMarkerSymbolLayer::dxfSize( const QgsDxfExport &e, QgsSymbolRenderContext &context ) const
{
double size = mSize;
Expand Down
7 changes: 7 additions & 0 deletions src/core/symbology/qgssymbollayer.h
Original file line number Diff line number Diff line change
Expand Up @@ -630,6 +630,13 @@ class CORE_EXPORT QgsSymbolLayer
*/
virtual QList<QgsSymbolLayerReference> masks() const;

/**
* Remove masks defined by this symbol layer.
* \see masks()
* \since QGIS 3.42
*/
virtual void clearMasks();

/**
* Prepares all mask internal objects according to what is defined in \a context
* This should be called prior to calling startRender() method.
Expand Down
20 changes: 20 additions & 0 deletions src/core/symbology/qgssymbollayerutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5605,6 +5605,26 @@ void QgsSymbolLayerUtils::resetSymbolLayerIds( QgsSymbolLayer *symbolLayer )
changeSymbolLayerIds( symbolLayer, []() { return QUuid::createUuid().toString(); } );
}

void QgsSymbolLayerUtils::clearSymbolLayerMasks( QgsSymbol *symbol )
{
if ( !symbol )
return;

for ( int idx = 0; idx < symbol->symbolLayerCount(); idx++ )
{
if ( QgsSymbolLayer *sl = symbol->symbolLayer( idx ) )
{
sl->clearMasks();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd lean toward a dynamic_cast to QgsMaskMarkerSymbolLayer here and then directly calling QgsMaskMarkerSymbolLayer::clearMasks to avoid the virtual clearMasks method in QgsSymbolLayer.

If later on we find other symbol layer types which need this handled then we can safely resurrect the virtual method without any API breakage.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered this solution but preferred to have clearMasks() method for 2 reasons:

  • to be consistent with masks() method (this is not the case of setMasks() but maybe this one should also be defined as a virtual method of QgsSymbolLayer)
  • dynamic_cast is a bad design and should be avoided

Even if we never get a symbol layer types implementing masks, why do you think it is better with dynamic_cast ?


// recurse over sub symbols
if ( QgsSymbol *subSymbol = sl->subSymbol() )
{
clearSymbolLayerMasks( subSymbol );
}
}
}
}

QVector<QgsGeometry> QgsSymbolLayerUtils::collectSymbolLayerClipGeometries( const QgsRenderContext &context, const QString &symbolLayerId, const QRectF &bounds )
{
QVector<QgsGeometry> clipGeometries = context.symbolLayerClipGeometries( symbolLayerId );
Expand Down
6 changes: 6 additions & 0 deletions src/core/symbology/qgssymbollayerutils.h
Original file line number Diff line number Diff line change
Expand Up @@ -949,6 +949,12 @@ class CORE_EXPORT QgsSymbolLayerUtils
*/
static void resetSymbolLayerIds( QgsSymbolLayer *symbolLayer );

/**
* Remove recursively masks from all \a symbol symbol layers
* \since QGIS 3.42
*/
static void clearSymbolLayerMasks( QgsSymbol *symbol );

/**
* Returns a list of the symbol layer clip geometries to be used for the symbol layer with the specified
* ID.
Expand Down
10 changes: 3 additions & 7 deletions src/gui/qgsmaskingwidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -264,25 +264,21 @@ bool SymbolLayerVisitor::visitEnter( const QgsStyleEntityVisitorInterface::Node
if ( node.type != QgsStyleEntityVisitorInterface::NodeType::SymbolRule )
return false;

mSymbolKey = node.identifier;
return true;
}

void SymbolLayerVisitor::visitSymbol( const QgsSymbol *symbol, const QString &leafIdentifier, QVector<int> rootPath )
void SymbolLayerVisitor::visitSymbol( const QgsSymbol *symbol, const QString &leafIdentifier )
{
for ( int idx = 0; idx < symbol->symbolLayerCount(); idx++ )
{
QVector<int> indexPath = rootPath;
indexPath.push_back( idx );

const QgsSymbolLayer *sl = symbol->symbolLayer( idx );

mCallback( sl, sl->id() );

// recurse over sub symbols
const QgsSymbol *subSymbol = const_cast<QgsSymbolLayer *>( sl )->subSymbol();
if ( subSymbol )
visitSymbol( subSymbol, leafIdentifier, indexPath );
visitSymbol( subSymbol, leafIdentifier );
}
}

Expand All @@ -292,7 +288,7 @@ bool SymbolLayerVisitor::visit( const QgsStyleEntityVisitorInterface::StyleLeaf
{
auto symbolEntity = static_cast<const QgsStyleSymbolEntity *>( leaf.entity );
if ( symbolEntity->symbol() )
visitSymbol( symbolEntity->symbol(), leaf.identifier, {} );
visitSymbol( symbolEntity->symbol(), leaf.identifier );
}
return true;
}
Expand Down
4 changes: 1 addition & 3 deletions src/gui/qgsmaskingwidget.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,11 @@ class SymbolLayerVisitor : public QgsStyleEntityVisitorInterface
bool visitEnter( const QgsStyleEntityVisitorInterface::Node &node ) override;

//! Process a symbol
void visitSymbol( const QgsSymbol *symbol, const QString &leafIdentifier, QVector<int> rootPath );
void visitSymbol( const QgsSymbol *symbol, const QString &leafIdentifier );

bool visit( const QgsStyleEntityVisitorInterface::StyleLeaf &leaf ) override;

private:
QString mSymbolKey;
QList<QPair<QgsSymbolLayerId, QList<QgsSymbolLayerReference>>> mMasks;
SymbolLayerCallback mCallback;
};

Expand Down
33 changes: 33 additions & 0 deletions tests/src/python/test_qgscategorizedsymbolrenderer.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,19 @@
QgsLineSymbol,
QgsMapSettings,
QgsMarkerSymbol,
QgsMaskMarkerSymbolLayer,
QgsProject,
QgsProperty,
QgsReadWriteContext,
QgsRectangle,
QgsRenderContext,
QgsRendererCategory,
QgsSimpleMarkerSymbolLayer,
QgsSingleSymbolRenderer,
QgsStyle,
QgsSymbol,
QgsSymbolLayer,
QgsSymbolLayerReference,
QgsVectorLayer,
)
import unittest
Expand Down Expand Up @@ -815,6 +818,36 @@ def testConvertFromEmbedded(self):
self.assertEqual(cc.label(), "")
self.assertEqual(cc.symbol().color().name(), "#ff00ff")

def testConvertNoMasks(self):
"""
Test converting an embedded symbol renderer to a categorized renderer
"""
points_layer = QgsVectorLayer("Point", "Polys", "memory")
f = QgsFeature()
f.setGeometry(QgsGeometry.fromWkt("Point(-100 30)"))
self.assertTrue(points_layer.dataProvider().addFeature(f))
f.setGeometry(QgsGeometry.fromWkt("Point(-110 40)"))
self.assertTrue(points_layer.dataProvider().addFeature(f))
f.setGeometry(QgsGeometry.fromWkt("Point(-90 50)"))
self.assertTrue(points_layer.dataProvider().addFeature(f))

p = QgsMarkerSymbol.createSimple({"color": "#fdbf6f", "size": "7"})
points_layer.setRenderer(QgsSingleSymbolRenderer(p))

circle_symbol = QgsMarkerSymbol.createSimple({"size": "10"})
mask_layer = QgsMaskMarkerSymbolLayer()
mask_layer.setSubSymbol(circle_symbol)
mask_layer.setMasks(
[QgsSymbolLayerReference("test_layer_id", "test_symbollayer_id")]
)
points_layer.renderer().symbol().appendSymbolLayer(mask_layer)

categorized = QgsCategorizedSymbolRenderer.convertFromRenderer(
points_layer.renderer(), points_layer
)

self.assertFalse(categorized.sourceSymbol().symbolLayers()[1].masks())

def test_displayString(self):
"""Test the displayString method"""

Expand Down
Loading