Fix rendering geometry generators for line layers

And add more tests
qgis · Dec 11, 2015 · c9fa334 · c9fa334 · nirvn · Dec 20, 2015
1 parent c176e3f
commit c9fa334
Show file tree

Hide file tree

Showing 7 changed files with 100 additions and 41 deletions.
diff --git a/src/core/qgsvectorlayer.h b/src/core/qgsvectorlayer.h
@@ -714,7 +714,10 @@ class CORE_EXPORT QgsVectorLayer : public QgsMapLayer
     /** Return const renderer V2. */
     const QgsFeatureRendererV2* rendererV2() const { return mRendererV2; }
 
-    /** Set renderer V2. */
+    /**
+     * Set renderer which will be invoked to represent this layer.
+     * Ownership is transferred.
+     */
     void setRendererV2( QgsFeatureRendererV2* r );
 
     /** Returns point, line or polygon */

diff --git a/src/core/symbology-ng/qgssymbolv2.cpp b/src/core/symbology-ng/qgssymbolv2.cpp
@@ -1227,26 +1227,29 @@ void QgsMarkerSymbolV2::renderPoint( const QPointF& point, const QgsFeature* f,
 
   if ( layerIdx != -1 )
   {
-    if ( layerIdx >= 0 && layerIdx < mLayers.count() )
+    QgsSymbolLayerV2* symbolLayer = mLayers.value( layerIdx );
+    if ( symbolLayer )
     {
-      QgsMarkerSymbolLayerV2* markerLayer = dynamic_cast<QgsMarkerSymbolLayerV2*>( mLayers.at( layerIdx ) );
-
-      if ( markerLayer )
+      if ( symbolLayer->type() == QgsSymbolV2::Marker )
+      {
+        QgsMarkerSymbolLayerV2* markerLayer = static_cast<QgsMarkerSymbolLayerV2*>( symbolLayer );
         renderPointUsingLayer( markerLayer, point, symbolContext );
+      }
       else
-        renderUsingLayer( mLayers.at( layerIdx ), symbolContext );
+        renderUsingLayer( symbolLayer, symbolContext );
     }
     return;
   }
 
-  Q_FOREACH ( QgsSymbolLayerV2* layer, mLayers )
+  Q_FOREACH ( QgsSymbolLayerV2* symbolLayer, mLayers )
   {
-    QgsMarkerSymbolLayerV2* markerLayer = dynamic_cast<QgsMarkerSymbolLayerV2*>( layer );
-
-    if ( markerLayer )
+    if ( symbolLayer->type() == QgsSymbolV2::Marker )
+    {
+      QgsMarkerSymbolLayerV2* markerLayer = static_cast<QgsMarkerSymbolLayerV2*>( symbolLayer );
       renderPointUsingLayer( markerLayer, point, symbolContext );
+    }
     else
-      renderUsingLayer( layer, symbolContext );
+      renderUsingLayer( symbolLayer, symbolContext );
   }
 }
 
@@ -1428,28 +1431,31 @@ void QgsLineSymbolV2::renderPolyline( const QPolygonF& points, const QgsFeature*
 
   if ( layerIdx != -1 )
   {
-    if ( layerIdx >= 0 && layerIdx < mLayers.count() )
+    QgsSymbolLayerV2* symbolLayer = mLayers.value( layerIdx );
+    if ( symbolLayer )
     {
-      QgsLineSymbolLayerV2* lineLayer = dynamic_cast<QgsLineSymbolLayerV2*>( mLayers.at( layerIdx ) );
-
-      if ( lineLayer )
+      if ( symbolLayer->type() == QgsSymbolV2::Line )
+      {
+        QgsLineSymbolLayerV2* lineLayer = static_cast<QgsLineSymbolLayerV2*>( symbolLayer );
         renderPolylineUsingLayer( lineLayer, points, symbolContext );
+      }
       else
-        renderUsingLayer( mLayers.at( layerIdx ), symbolContext );
+        renderUsingLayer( symbolLayer, symbolContext );
     }
     return;
   }
 
   Q_FOREACH ( QgsSymbolLayerV2* symbolLayer, mLayers )
   {
-    if ( symbolLayer->type() != QgsSymbolV2::Line )
-      continue;
-    QgsLineSymbolLayerV2* lineLayer = static_cast<QgsLineSymbolLayerV2*>( symbolLayer );
-
-    if ( lineLayer )
+    if ( symbolLayer->type() == QgsSymbolV2::Line )
+    {
+      QgsLineSymbolLayerV2* lineLayer = static_cast<QgsLineSymbolLayerV2*>( symbolLayer );
       renderPolylineUsingLayer( lineLayer, points, symbolContext );
+    }
     else
+    {
       renderUsingLayer( symbolLayer, symbolContext );
+    }
   }
 
   context.setPainter( renderPainter );
@@ -1502,23 +1508,23 @@ void QgsFillSymbolV2::renderPolygon( const QPolygonF& points, QList<QPolygonF>*
 
   if ( layerIdx != -1 )
   {
-    QgsSymbolLayerV2* layer = mLayers.value( layerIdx );
-    if ( layer )
+    QgsSymbolLayerV2* symbolLayer = mLayers.value( layerIdx );
+    if ( symbolLayer )
     {
-      if ( layer->type() == Fill || layer->type() == Line )
-        renderPolygonUsingLayer( layer, points, rings, symbolContext );
+      if ( symbolLayer->type() == Fill || symbolLayer->type() == Line )
+        renderPolygonUsingLayer( symbolLayer, points, rings, symbolContext );
       else
-        renderUsingLayer( layer, symbolContext );
+        renderUsingLayer( symbolLayer, symbolContext );
     }
     return;
   }
 
-  Q_FOREACH ( QgsSymbolLayerV2* layer, mLayers )
+  Q_FOREACH ( QgsSymbolLayerV2* symbolLayer, mLayers )
   {
-    if ( layer->type() == Fill || layer->type() == Line )
-      renderPolygonUsingLayer( layer, points, rings, symbolContext );
+    if ( symbolLayer->type() == Fill || symbolLayer->type() == Line )
+      renderPolygonUsingLayer( symbolLayer, points, rings, symbolContext );
     else
-      renderUsingLayer( layer, symbolContext );
+      renderUsingLayer( symbolLayer, symbolContext );
   }
 }
 

diff --git a/tests/src/python/test_qgsgeometrygeneratorsymbollayerv2.py b/tests/src/python/test_qgsgeometrygeneratorsymbollayerv2.py
@@ -31,6 +31,7 @@
 from qgis.core import (QgsVectorLayer,
                        QgsSingleSymbolRendererV2,
                        QgsFillSymbolV2,
+                       QgsLineSymbolV2,
                        QgsMarkerSymbolV2,
                        QgsMapLayerRegistry,
                        QgsRectangle,
@@ -53,49 +54,98 @@
 class TestQgsGeometryGeneratorSymbolLayerV2(TestCase):
 
     def setUp(self):
-        myShpFile = os.path.join(TEST_DATA_DIR, 'polys_overlapping.shp')
-        layer = QgsVectorLayer(myShpFile, 'Polygons', 'ogr')
-        QgsMapLayerRegistry.instance().addMapLayer(layer)
+        polys_shp = os.path.join(TEST_DATA_DIR, 'polys.shp')
+        points_shp = os.path.join(TEST_DATA_DIR, 'points.shp')
+        lines_shp = os.path.join(TEST_DATA_DIR, 'lines.shp')
+        self.polys_layer = QgsVectorLayer(polys_shp, 'Polygons', 'ogr')
+        self.points_layer = QgsVectorLayer(points_shp, 'Points', 'ogr')
+        self.lines_layer = QgsVectorLayer(lines_shp, 'Lines', 'ogr')
+        QgsMapLayerRegistry.instance().addMapLayer(self.polys_layer)
+        QgsMapLayerRegistry.instance().addMapLayer(self.lines_layer)
+        QgsMapLayerRegistry.instance().addMapLayer(self.points_layer)
 
         # Create style
         sym1 = QgsFillSymbolV2.createSimple({'color': '#fdbf6f'})
+        sym2 = QgsLineSymbolV2.createSimple({'color': '#fdbf6f'})
+        sym3 = QgsMarkerSymbolV2.createSimple({'color': '#fdbf6f'})
+
+        self.polys_layer.setRendererV2(QgsSingleSymbolRendererV2(sym1))
+        self.lines_layer.setRendererV2(QgsSingleSymbolRendererV2(sym2))
+        self.points_layer.setRendererV2(QgsSingleSymbolRendererV2(sym3))
 
-        self.renderer = QgsSingleSymbolRendererV2(sym1)
-        layer.setRendererV2(self.renderer)
         self.mapsettings = CANVAS.mapSettings()
         self.mapsettings.setOutputSize(QSize(400, 400))
         self.mapsettings.setOutputDpi(96)
         self.mapsettings.setExtent(QgsRectangle(-133, 22, -70, 52))
 
-        rendered_layers = [layer.id()]
-        self.mapsettings.setLayers(rendered_layers)
-
     def tearDown(self):
         QgsMapLayerRegistry.instance().removeAllMapLayers()
 
     def test_marker(self):
-        sym = self.renderer.symbol()
+        sym = self.polys_layer.rendererV2().symbol()
         sym_layer = QgsGeometryGeneratorSymbolLayerV2.create({'geometryModifier': 'centroid($geometry)'})
         sym_layer.setSymbolType(QgsSymbolV2.Marker)
         sym.changeSymbolLayer(0, sym_layer)
+
+        rendered_layers = [self.polys_layer.id()]
+        self.mapsettings.setLayers(rendered_layers)
+
         renderchecker = QgsMultiRenderChecker()
         renderchecker.setMapSettings(self.mapsettings)
         renderchecker.setControlName('expected_geometrygenerator_marker')
         self.assertTrue(renderchecker.runTest('geometrygenerator_marker'))
 
     def test_mixed(self):
-        sym = self.renderer.symbol()
+        sym = self.polys_layer.rendererV2().symbol()
+
         buffer_layer = QgsGeometryGeneratorSymbolLayerV2.create({'geometryModifier': 'buffer($geometry, "value"/15)'})
         buffer_layer.setSymbolType(QgsSymbolV2.Fill)
-        buffer_layer.subSymbol()
+        self.assertIsNotNone(buffer_layer.subSymbol())
         sym.appendSymbolLayer(buffer_layer)
         marker_layer = QgsGeometryGeneratorSymbolLayerV2.create({'geometryModifier': 'centroid($geometry)'})
         marker_layer.setSymbolType(QgsSymbolV2.Marker)
         sym.appendSymbolLayer(marker_layer)
+
+        rendered_layers = [self.polys_layer.id()]
+        self.mapsettings.setLayers(rendered_layers)
+
         renderchecker = QgsMultiRenderChecker()
         renderchecker.setMapSettings(self.mapsettings)
         renderchecker.setControlName('expected_geometrygenerator_mixed')
         self.assertTrue(renderchecker.runTest('geometrygenerator_mixed'))
 
+    def test_buffer_lines(self):
+        sym = self.lines_layer.rendererV2().symbol()
+
+        buffer_layer = QgsGeometryGeneratorSymbolLayerV2.create({'geometryModifier': 'buffer($geometry, "value"/15)'})
+        buffer_layer.setSymbolType(QgsSymbolV2.Fill)
+        self.assertIsNotNone(buffer_layer.subSymbol())
+        sym.appendSymbolLayer(buffer_layer)
+
+        rendered_layers = [self.lines_layer.id()]
+        self.mapsettings.setLayers(rendered_layers)
+
+        renderchecker = QgsMultiRenderChecker()
+        renderchecker.setMapSettings(self.mapsettings)
+        renderchecker.setControlName('expected_geometrygenerator_buffer_lines')
+        self.assertTrue(renderchecker.runTest('geometrygenerator_buffer_lines'))
+
+    def test_buffer_points(self):
+        sym = self.points_layer.rendererV2().symbol()
+
+        buffer_layer = QgsGeometryGeneratorSymbolLayerV2.create({'geometryModifier': 'buffer($geometry, "staff"/15)'})
+        buffer_layer.setSymbolType(QgsSymbolV2.Fill)
+        self.assertIsNotNone(buffer_layer.subSymbol())
+        sym.appendSymbolLayer(buffer_layer)
+
+        rendered_layers = [self.points_layer.id()]
+        self.mapsettings.setLayers(rendered_layers)
+
+        renderchecker = QgsMultiRenderChecker()
+        renderchecker.setMapSettings(self.mapsettings)
+        renderchecker.setControlName('expected_geometrygenerator_buffer_points')
+        self.assertTrue(renderchecker.runTest('geometrygenerator_buffer_points'))
+
+
 if __name__ == '__main__':
     unittest.main()
diff --git a/...cted_geometrygenerator_buffer_lines/expected_geometrygenerator_buffer_lines.png b/...cted_geometrygenerator_buffer_lines/expected_geometrygenerator_buffer_lines.png
diff --git a/...ed_geometrygenerator_buffer_points/expected_geometrygenerator_buffer_points.png b/...ed_geometrygenerator_buffer_points/expected_geometrygenerator_buffer_points.png
diff --git a/..._images/expected_geometrygenerator_marker/expected_geometrygenerator_marker.png b/..._images/expected_geometrygenerator_marker/expected_geometrygenerator_marker.png
diff --git a/...ol_images/expected_geometrygenerator_mixed/expected_geometrygenerator_mixed.png b/...ol_images/expected_geometrygenerator_mixed/expected_geometrygenerator_mixed.png