Merge pull request #50200 from troopa81/fix_legend_geomgenerator

Fix legend icon geom generator is involved
qgis · Sep 27, 2022 · 2d1e3fb · 2d1e3fb
2 parents c7306ab + dcf64e8
commit 2d1e3fb
Show file tree

Hide file tree

Showing 7 changed files with 149 additions and 37 deletions.
diff --git a/python/core/auto_generated/symbology/qgssymbol.sip.in b/python/core/auto_generated/symbology/qgssymbol.sip.in
@@ -125,7 +125,7 @@ Returns the symbol's type.
 %End
 
 
-    QgsSymbolLayerList symbolLayers();
+    QgsSymbolLayerList symbolLayers() const;
 %Docstring
 Returns the list of symbol layers contained in the symbol.
 
@@ -780,7 +780,6 @@ Render editing vertex marker at specified point
     QgsSymbol( const QgsSymbol & );
 };
 
-
 /************************************************************************
  * This file has been generated automatically from                      *
  *                                                                      *

diff --git a/python/core/auto_generated/symbology/qgssymbollayerutils.sip.in b/python/core/auto_generated/symbology/qgssymbollayerutils.sip.in
@@ -971,7 +971,7 @@ Returns -1 if the ``renderer`` is not animated.
 .. versionadded:: 3.26
 %End
 
-    static QgsSymbol *restrictedSizeSymbol( const QgsSymbol *s, double minSize, double maxSize, QgsRenderContext *context, double &width, double &height );
+    static QgsSymbol *restrictedSizeSymbol( const QgsSymbol *s, double minSize, double maxSize, QgsRenderContext *context, double &width, double &height, bool *ok = 0 );
 %Docstring
 Creates a new symbol with size restricted to min/max size if original size is out of min/max range
 
@@ -980,9 +980,13 @@ Creates a new symbol with size restricted to min/max size if original size is ou
 :param maxSize: the maximum size in mm
 :param context: the render context
 :param width: expected width, can be changed by the function
-:param height: expected height, can be changed by this function
+:param height: expected height, can be changed by the function
+:param ok: if not None, ok is set to false if it's not possible to compute a restricted symbol (if geometry generators
+           are involved for instance)
 
-:return: 0 if size is within minSize/maxSize range. New symbol if size was out of min/max range. Caller takes ownership
+:return: None if size is within minSize/maxSize range or if it's not possible to compute a
+         restricted size symbol. New symbol if size was out of min/max range.
+         Caller takes ownership
 %End
 
     static QgsStringMap evaluatePropertiesMap( const QMap<QString, QgsProperty> &propertiesMap, const QgsExpressionContext &context );
@@ -997,8 +1001,6 @@ Evaluates a map of properties using the given ``context`` and returns a variant
 
 
 
-
-
 /************************************************************************
  * This file has been generated automatically from                      *
  *                                                                      *

diff --git a/src/core/layertree/qgslayertreemodellegendnode.cpp b/src/core/layertree/qgslayertreemodellegendnode.cpp
@@ -37,6 +37,7 @@
 #include "qgsmarkersymbol.h"
 #include "qgsvariantutils.h"
 #include "qgslayertreelayer.h"
+#include "qgsgeometrygeneratorsymbollayer.h"
 
 #include <QBuffer>
 
@@ -343,25 +344,29 @@ QSize QgsSymbolLegendNode::minimumIconSize( QgsRenderContext *context ) const
   const int iconSize = QgsLayerTreeModel::scaleIconSize( 16 );
   const int largeIconSize = QgsLayerTreeModel::scaleIconSize( 512 );
   QSize minSz( iconSize, iconSize );
-  if ( mItem.symbol() && mItem.symbol()->type() == Qgis::SymbolType::Marker )
+  if ( mItem.symbol() && ( mItem.symbol()->type() == Qgis::SymbolType::Marker
+                           || mItem.symbol()->type() == Qgis::SymbolType::Line ) )
   {
+    int maxSize = largeIconSize;
+
     // unusued width, height variables
     double width = 0.0;
     double height = 0.0;
-    const std::unique_ptr<QgsSymbol> symbol( QgsSymbolLayerUtils::restrictedSizeSymbol( mItem.symbol(), MINIMUM_SIZE, MAXIMUM_SIZE, context, width, height ) );
-    minSz = QgsImageOperation::nonTransparentImageRect(
-              QgsSymbolLayerUtils::symbolPreviewPixmap( symbol ? symbol.get() : mItem.symbol(), QSize( largeIconSize, largeIconSize ), 0,
-                  context ).toImage(),
-              minSz,
-              true ).size();
-  }
-  else if ( mItem.symbol() && mItem.symbol()->type() == Qgis::SymbolType::Line )
-  {
-    double width = 0.0;
-    double height = 0.0;
-    const std::unique_ptr<QgsSymbol> symbol( QgsSymbolLayerUtils::restrictedSizeSymbol( mItem.symbol(), MINIMUM_SIZE, MAXIMUM_SIZE, context, width, height ) );
+    bool ok;
+    std::unique_ptr<QgsSymbol> symbol( QgsSymbolLayerUtils::restrictedSizeSymbol( mItem.symbol(), MINIMUM_SIZE, MAXIMUM_SIZE, context, width, height, &ok ) );
+
+    if ( !ok && context )
+    {
+      // It's not possible to get a restricted size symbol, so we restrict
+      // pixmap target size to be sure it would fit MAXIMUM_SIZE
+      maxSize = static_cast<int>( std::round( MAXIMUM_SIZE * context->scaleFactor() ) );
+    }
+
+    const QSize size( mItem.symbol()->type() == Qgis::SymbolType::Marker ? maxSize : minSz.width(),
+                      maxSize );
+
     minSz = QgsImageOperation::nonTransparentImageRect(
-              QgsSymbolLayerUtils::symbolPreviewPixmap( symbol ? symbol.get() : mItem.symbol(), QSize( minSz.width(), largeIconSize ), 0,
+              QgsSymbolLayerUtils::symbolPreviewPixmap( symbol ? symbol.get() : mItem.symbol(), size, 0,
                   context ).toImage(),
               minSz,
               true ).size();
@@ -1564,4 +1569,3 @@ void QgsVectorLabelLegendNode::textWidthHeight( double &width, double &height, Q
   height = QgsTextRenderer::textHeight( ctx, textFormat, 'A', true );
   width = QgsTextRenderer::textWidth( ctx, textFormat, textLines, &fm );
 }
-
diff --git a/src/core/symbology/qgssymbol.h b/src/core/symbology/qgssymbol.h
@@ -160,7 +160,7 @@ class CORE_EXPORT QgsSymbol
      * \see symbolLayerCount
      * \since QGIS 2.7
      */
-    QgsSymbolLayerList symbolLayers() { return mLayers; }
+    QgsSymbolLayerList symbolLayers() const { return mLayers; }
 
 #ifndef SIP_RUN
 
@@ -856,4 +856,3 @@ class CORE_EXPORT QgsSymbol
 };
 
 #endif
-
diff --git a/src/core/symbology/qgssymbollayerutils.cpp b/src/core/symbology/qgssymbollayerutils.cpp
@@ -4983,18 +4983,34 @@ double QgsSymbolLayerUtils::rendererFrameRate( const QgsFeatureRenderer *rendere
   return visitor.refreshRate;
 }
 
-QgsSymbol *QgsSymbolLayerUtils::restrictedSizeSymbol( const QgsSymbol *s, double minSize, double maxSize, QgsRenderContext *context, double &width, double &height )
+QgsSymbol *QgsSymbolLayerUtils::restrictedSizeSymbol( const QgsSymbol *s, double minSize, double maxSize, QgsRenderContext *context, double &width, double &height, bool *ok )
 {
   if ( !s || !context )
   {
-    return 0;
+    return nullptr;
   }
 
+  if ( ok )
+    *ok = true;
+
   double size;
   const QgsMarkerSymbol *markerSymbol = dynamic_cast<const QgsMarkerSymbol *>( s );
   const QgsLineSymbol *lineSymbol = dynamic_cast<const QgsLineSymbol *>( s );
   if ( markerSymbol )
   {
+    const QgsSymbolLayerList sls = s->symbolLayers();
+    for ( const QgsSymbolLayer *sl : std::as_const( sls ) )
+    {
+      // geometry generators involved, there is no way to get a restricted size symbol
+      if ( sl->type() != Qgis::SymbolType::Marker )
+      {
+        if ( ok )
+          *ok = false;
+
+        return nullptr;
+      }
+    }
+
     size = markerSymbol->size( *context );
   }
   else if ( lineSymbol )
@@ -5003,7 +5019,10 @@ QgsSymbol *QgsSymbolLayerUtils::restrictedSizeSymbol( const QgsSymbol *s, double
   }
   else
   {
-    return 0; //not size restriction implemented for other symbol types
+    if ( ok )
+      *ok = false;
+
+    return nullptr; //not size restriction implemented for other symbol types
   }
 
   size /= context->scaleFactor();
@@ -5018,7 +5037,8 @@ QgsSymbol *QgsSymbolLayerUtils::restrictedSizeSymbol( const QgsSymbol *s, double
   }
   else
   {
-    return 0;
+    // no need to restricted size symbol
+    return nullptr;
   }
 
   if ( markerSymbol )
@@ -5038,7 +5058,8 @@ QgsSymbol *QgsSymbolLayerUtils::restrictedSizeSymbol( const QgsSymbol *s, double
     height = size;
     return ls;
   }
-  return 0;
+
+  return nullptr;
 }
 
 QgsStringMap QgsSymbolLayerUtils::evaluatePropertiesMap( const QMap<QString, QgsProperty> &propertiesMap, const QgsExpressionContext &context )
@@ -5051,4 +5072,3 @@ QgsStringMap QgsSymbolLayerUtils::evaluatePropertiesMap( const QMap<QString, Qgs
   }
   return properties;
 }
-
diff --git a/src/core/symbology/qgssymbollayerutils.h b/src/core/symbology/qgssymbollayerutils.h
@@ -881,10 +881,14 @@ class CORE_EXPORT QgsSymbolLayerUtils
      * \param maxSize the maximum size in mm
      * \param context the render context
      * \param width expected width, can be changed by the function
-     * \param height expected height, can be changed by this function
-     * \return 0 if size is within minSize/maxSize range. New symbol if size was out of min/max range. Caller takes ownership
+     * \param height expected height, can be changed by the function
+     * \param ok if not nullptr, ok is set to false if it's not possible to compute a restricted symbol (if geometry generators
+     * are involved for instance)
+     * \return nullptr if size is within minSize/maxSize range or if it's not possible to compute a
+     * restricted size symbol. New symbol if size was out of min/max range.
+     * Caller takes ownership
      */
-    static QgsSymbol *restrictedSizeSymbol( const QgsSymbol *s, double minSize, double maxSize, QgsRenderContext *context, double &width, double &height );
+    static QgsSymbol *restrictedSizeSymbol( const QgsSymbol *s, double minSize, double maxSize, QgsRenderContext *context, double &width, double &height, bool *ok = nullptr );
 
     /**
      * Evaluates a map of properties using the given \a context and returns a variant map with evaluated expressions from the properties.
@@ -925,5 +929,3 @@ class QPolygonF;
 QList<QPolygonF> offsetLine( QPolygonF polyline, double dist, QgsWkbTypes::GeometryType geometryType ) SIP_SKIP;
 
 #endif
-
-
diff --git a/tests/src/core/testqgslayertree.cpp b/tests/src/core/testqgslayertree.cpp
@@ -31,6 +31,9 @@
 #include "qgslegendsettings.h"
 #include "qgsmarkersymbol.h"
 #include "qgsannotationlayer.h"
+#include "qgsgeometrygeneratorsymbollayer.h"
+#include "qgsfillsymbol.h"
+#include "qgsfillsymbollayer.h"
 #include <QSignalSpy>
 
 class TestQgsLayerTree : public QObject
@@ -46,7 +49,10 @@ class TestQgsLayerTree : public QObject
     void testCheckStateHiearchical();
     void testCheckStateMutuallyExclusive();
     void testCheckStateMutuallyExclusiveEdgeCases();
+    void testRestrictedSymbolSize_data();
     void testRestrictedSymbolSize();
+    void testRestrictedSymbolSizeWithGeometryGenerator_data();
+    void testRestrictedSymbolSizeWithGeometryGenerator();
     void testShowHideAllSymbolNodes();
     void testFindLegendNode();
     void testLegendSymbolCategorized();
@@ -304,10 +310,26 @@ void TestQgsLayerTree::testCheckStateMutuallyExclusiveEdgeCases()
   delete root3;
 }
 
+void TestQgsLayerTree::testRestrictedSymbolSize_data()
+{
+  QTest::addColumn<double>( "maxSize" );
+  QTest::addColumn<int>( "expectedSize" );
+
+  // QTest::newRow( "smaller than max" ) << 15. << 52;
+  QTest::newRow( "bigger than max" ) << 10. << 40;
+}
+
 void TestQgsLayerTree::testRestrictedSymbolSize()
 {
+  QFETCH( double, maxSize );
+  QFETCH( int, expectedSize );
+
+  // to force re-read of max/min size in QgsSymbolLegendNode constructor
+  QgsSymbolLegendNode::MINIMUM_SIZE = -1;
+  QgsSymbolLegendNode::MAXIMUM_SIZE = -1;
+
   QgsSettings settings;
-  settings.setValue( "/qgis/legendsymbolMaximumSize", 15.0 );
+  settings.setValue( "/qgis/legendsymbolMaximumSize", maxSize );
 
   //new memory layer
   QgsVectorLayer *vl = new QgsVectorLayer( QStringLiteral( "Point?field=col1:integer" ), QStringLiteral( "vl" ), QStringLiteral( "memory" ) );
@@ -337,7 +359,71 @@ void TestQgsLayerTree::testRestrictedSymbolSize()
 
   const QList<QgsLayerTreeModelLegendNode *> nodes = m->layerLegendNodes( n );
   const QSize minimumSize = static_cast< QgsSymbolLegendNode *>( nodes.at( 0 ) )->minimumIconSize();
-  QCOMPARE( minimumSize.width(), 52 );
+  QCOMPARE( minimumSize.width(), expectedSize );
+
+  //cleanup
+  delete m;
+  delete root;
+}
+
+void TestQgsLayerTree::testRestrictedSymbolSizeWithGeometryGenerator_data()
+{
+  QTest::addColumn<double>( "maxSize" );
+  QTest::addColumn<int>( "expectedSize" );
+
+  QTest::newRow( "smaller than max" ) << 15. << 42;
+  QTest::newRow( "bigger than max" ) << 10. << 38;
+}
+
+void TestQgsLayerTree::testRestrictedSymbolSizeWithGeometryGenerator()
+{
+  QFETCH( double, maxSize );
+  QFETCH( int, expectedSize );
+
+  // to force re-read of max/min size in QgsSymbolLegendNode constructor
+  QgsSymbolLegendNode::MINIMUM_SIZE = -1;
+  QgsSymbolLegendNode::MAXIMUM_SIZE = -1;
+
+  QgsSettings settings;
+  settings.setValue( "/qgis/legendsymbolMaximumSize", maxSize );
+
+  //new memory layer
+  QgsVectorLayer *vl = new QgsVectorLayer( QStringLiteral( "Point?field=col1:integer" ), QStringLiteral( "vl" ), QStringLiteral( "memory" ) );
+  QVERIFY( vl->isValid() );
+
+  QgsProject project;
+  project.addMapLayer( vl );
+
+  //create a categorized renderer with geometry generator for layer
+
+  QVariantMap ggProps;
+  ggProps.insert( QStringLiteral( "SymbolType" ), QStringLiteral( "Fill" ) );
+  ggProps.insert( QStringLiteral( "geometryModifier" ), QStringLiteral( "buffer( $geometry, 200 )" ) );
+  QgsSymbolLayer *ggSymbolLayer = QgsGeometryGeneratorSymbolLayer::create( ggProps );
+  QgsSymbolLayerList fillSymbolLayerList;
+  fillSymbolLayerList << new QgsSimpleFillSymbolLayer();
+  ggSymbolLayer->setSubSymbol( new QgsFillSymbol( fillSymbolLayerList ) );
+  QgsSymbolLayerList slList;
+  slList << ggSymbolLayer;
+  QgsMarkerSymbol *symbol = new QgsMarkerSymbol( slList );
+
+  QgsCategorizedSymbolRenderer *renderer = new QgsCategorizedSymbolRenderer();
+  renderer->setClassAttribute( QStringLiteral( "col1" ) );
+  renderer->setSourceSymbol( symbol->clone() );
+  renderer->addCategory( QgsRendererCategory( "a", symbol->clone(), QStringLiteral( "a" ) ) );
+  renderer->addCategory( QgsRendererCategory( "b", symbol->clone(), QStringLiteral( "b" ) ) );
+  vl->setRenderer( renderer );
+
+  //create legend with symbology nodes for categorized renderer
+  QgsLayerTree *root = new QgsLayerTree();
+  QgsLayerTreeLayer *n = new QgsLayerTreeLayer( vl );
+  root->addChildNode( n );
+  QgsLayerTreeModel *m = new QgsLayerTreeModel( root, nullptr );
+  m->setLegendMapViewData( 10, 96, 10 );
+
+  const QList<QgsLayerTreeModelLegendNode *> nodes = m->layerLegendNodes( n );
+  const QSize minimumSize = static_cast< QgsSymbolLegendNode *>( nodes.at( 0 ) )->minimumIconSize();
+  QCOMPARE( minimumSize.width(), expectedSize );
 
   //cleanup
   delete m;