Newly created random marker fills should default to a new random seed,

rather then the "randomized" seed setting Having seeded random fills by default is much more predicatable and expected behavior for users
qgis · Oct 28, 2019 · f373203 · f373203
1 parent 5aa6d65
commit f373203
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 1 deletion.
diff --git a/src/core/symbology/qgsfillsymbollayer.cpp b/src/core/symbology/qgsfillsymbollayer.cpp
@@ -39,6 +39,7 @@
 #include <QSvgRenderer>
 #include <QDomDocument>
 #include <QDomElement>
+#include <random>
 
 QgsSimpleFillSymbolLayer::QgsSimpleFillSymbolLayer( const QColor &color, Qt::BrushStyle style, const QColor &strokeColor, Qt::PenStyle strokeStyle, double strokeWidth,
     Qt::PenJoinStyle penJoinStyle )
@@ -3927,7 +3928,20 @@ QgsRandomMarkerFillSymbolLayer::QgsRandomMarkerFillSymbolLayer( int pointCount,
 QgsSymbolLayer *QgsRandomMarkerFillSymbolLayer::create( const QgsStringMap &properties )
 {
   const int pointCount = properties.value( QStringLiteral( "point_count" ), QStringLiteral( "1" ) ).toInt();
-  const unsigned long seed = properties.value( QStringLiteral( "seed" ), QStringLiteral( "0" ) ).toULong();
+
+  unsigned long seed = 0;
+  if ( properties.contains( QStringLiteral( "seed" ) ) )
+    seed = properties.value( QStringLiteral( "seed" ), QStringLiteral( "0" ) ).toULong();
+  else
+  {
+    // if we a creating a new random marker fill from scratch, we default to a random seed
+    // because seed based fills are just nicer for users vs seeing points jump around with every map refresh
+    std::random_device rd;
+    std::mt19937 mt( seed == 0 ? rd() : seed );
+    std::uniform_int_distribution<> uniformDist( 1, 999999999 );
+    seed = uniformDist( mt );
+  }
+
   std::unique_ptr< QgsRandomMarkerFillSymbolLayer > sl = qgis::make_unique< QgsRandomMarkerFillSymbolLayer >( pointCount, seed );
 
   if ( properties.contains( QStringLiteral( "clip_points" ) ) )

diff --git a/tests/src/python/test_qgsrandommarkersymbollayer.py b/tests/src/python/test_qgsrandommarkersymbollayer.py
@@ -127,6 +127,21 @@ def testSimple(self):
         rendered_image = self.renderGeometry(s3, g)
         self.assertFalse(self.imageCheck('randommarkerfill_seed', 'randommarkerfill_seed', rendered_image))
 
+    def testCreate(self):
+        random_fill = QgsRandomMarkerFillSymbolLayer(10, seed=5)
+        self.assertEqual(random_fill.seed(), 5)
+        fill2 = QgsRandomMarkerFillSymbolLayer.create(random_fill.properties())
+        self.assertEqual(fill2.seed(), 5)
+
+        random_fill = QgsRandomMarkerFillSymbolLayer(10)
+        self.assertEqual(random_fill.seed(), 0)
+        fill2 = QgsRandomMarkerFillSymbolLayer.create(random_fill.properties())
+        self.assertEqual(fill2.seed(), 0)
+
+        # a newly created random fill should default to a random seed, not 0
+        random_fill = QgsRandomMarkerFillSymbolLayer.create({})
+        self.assertNotEqual(random_fill.seed(), 0)
+
     def renderGeometry(self, symbol, geom, buffer=20):
         f = QgsFeature()
         f.setGeometry(geom)