Merge pull request #33292 from m-kuhn/legend_marker_symbol_field_depe…

…ndent_3_10 [backport release-3_10] When DD rotation fails to evaluate, fallback to static value
qgis · Dec 9, 2019 · ffe31ee · ffe31ee
2 parents ef24c52 + 5cd8b0a
commit ffe31ee
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 3 deletions.
diff --git a/src/core/symbology/qgsmarkersymbollayer.cpp b/src/core/symbology/qgsmarkersymbollayer.cpp
@@ -645,18 +645,24 @@ void QgsSimpleMarkerSymbolLayerBase::calculateOffsetAndRotation( QgsSymbolRender
   markerOffset( context, scaledSize, scaledSize, offsetX, offsetY );
   offset = QPointF( offsetX, offsetY );
 
+  hasDataDefinedRotation = false;
   //angle
   bool ok = true;
   angle = mAngle + mLineAngle;
-  bool usingDataDefinedRotation = false;
   if ( mDataDefinedProperties.isActive( QgsSymbolLayer::PropertyAngle ) )
   {
     context.setOriginalValueVariable( angle );
     angle = mDataDefinedProperties.valueAsDouble( QgsSymbolLayer::PropertyAngle, context.renderContext().expressionContext(), mAngle, &ok ) + mLineAngle;
-    usingDataDefinedRotation = ok;
+
+    // If the expression evaluation was not successful, fallback to static value
+    if ( !ok )
+      angle = mAngle + mLineAngle;
+
+    hasDataDefinedRotation = true;
   }
 
-  hasDataDefinedRotation = context.renderHints() & QgsSymbol::DynamicRotation || usingDataDefinedRotation;
+  hasDataDefinedRotation = context.renderHints() & QgsSymbol::DynamicRotation || hasDataDefinedRotation;
+
   if ( hasDataDefinedRotation )
   {
     // For non-point markers, "dataDefinedRotation" means following the

diff --git a/tests/src/core/testqgssimplemarker.cpp b/tests/src/core/testqgssimplemarker.cpp
@@ -35,6 +35,23 @@
 //qgis test includes
 #include "qgsrenderchecker.h"
 
+static QString _fileNameForTest( const QString &testName )
+{
+  return QDir::tempPath() + '/' + testName + ".png";
+}
+
+static bool _verifyImage( const QString &testName, QString &report )
+{
+  QgsRenderChecker checker;
+  checker.setControlPathPrefix( QStringLiteral( "qgssimplemarkertest" ) );
+  checker.setControlName( "expected_" + testName );
+  checker.setRenderedImage( _fileNameForTest( testName ) );
+  checker.setSizeTolerance( 3, 3 );
+  bool equal = checker.compareImages( testName, 500 );
+  report += checker.report();
+  return equal;
+}
+
 /**
  * \ingroup UnitTests
  * This is a unit test for simple marker symbol types.
@@ -54,6 +71,8 @@ class TestQgsSimpleMarkerSymbol : public QObject
 
     void simpleMarkerSymbol();
     void simpleMarkerSymbolRotation();
+    void simpleMarkerSymbolPreviewRotation();
+    void simpleMarkerSymbolPreviewRotation_data();
     void simpleMarkerSymbolBevelJoin();
     void simpleMarkerSymbolMiterJoin();
     void simpleMarkerSymbolRoundJoin();
@@ -156,6 +175,37 @@ void TestQgsSimpleMarkerSymbol::simpleMarkerSymbolRotation()
   QVERIFY( imageCheck( "simplemarker_rotation" ) );
 }
 
+void TestQgsSimpleMarkerSymbol::simpleMarkerSymbolPreviewRotation()
+{
+  QFETCH( QString, name );
+  QFETCH( double, angle );
+  QFETCH( QString, expression );
+  QgsMarkerSymbol markerSymbol;
+  QgsSimpleMarkerSymbolLayer *simpleMarkerLayer = new QgsSimpleMarkerSymbolLayer();
+  markerSymbol.changeSymbolLayer( 0, simpleMarkerLayer );
+
+  simpleMarkerLayer->setShape( QgsSimpleMarkerSymbolLayerBase::Shape::Arrow );
+  simpleMarkerLayer->setAngle( angle );
+  simpleMarkerLayer->setSize( 20 );
+  simpleMarkerLayer->setColor( Qt::red );
+  simpleMarkerLayer->setDataDefinedProperty( QgsSymbolLayer::PropertyAngle, QgsProperty::fromExpression( expression ) );
+
+  QgsExpressionContext ec;
+  QImage image = markerSymbol.bigSymbolPreviewImage( &ec );
+  image.save( _fileNameForTest( name ) );
+  QVERIFY( _verifyImage( name, mReport ) );
+}
+
+void TestQgsSimpleMarkerSymbol::simpleMarkerSymbolPreviewRotation_data()
+{
+  QTest::addColumn<QString>( "name" );
+  QTest::addColumn<double>( "angle" );
+  QTest::addColumn<QString>( "expression" );
+
+  QTest::newRow( "field_based" ) << QStringLiteral( "field_based" ) << 20. << QStringLiteral( "orientation" ); // Should fallback to 20 because orientation is not available
+  QTest::newRow( "static_expression" ) << QStringLiteral( "static_expression" ) << 20. << QStringLiteral( "40" ); // Should use 40 because expression has precedence
+}
+
 void TestQgsSimpleMarkerSymbol::simpleMarkerSymbolBevelJoin()
 {
   mReport += QLatin1String( "<h2>Simple marker symbol layer test</h2>\n" );

diff --git a/...ontrol_images/qgssimplemarkertest/expected_field_based/expected_field_based.png b/...ontrol_images/qgssimplemarkertest/expected_field_based/expected_field_based.png
diff --git a/...s/qgssimplemarkertest/expected_static_expression/expected_static_expression.png b/...s/qgssimplemarkertest/expected_static_expression/expected_static_expression.png