Fix saving of "obstacle only" state + misc fixes to unit tests

qgis · May 16, 2017 · f153e19 · f153e19
1 parent bfdc84f
commit f153e19
Show file tree

Hide file tree

Showing 14 changed files with 45 additions and 67 deletions.
diff --git a/doc/api_break.dox b/doc/api_break.dox
@@ -1667,6 +1667,7 @@ QgsPalLabeling        {#qgis_api_break_3_0_QgsPalLabeling}
 QgsPalLayerSettings        {#qgis_api_break_3_0_QgsPalLayerSettings}
 -------------------
 
+- "enabled" member variable has been removed. Labeling is enabled if layer.labeling() does not return null pointer. To disable labeling, call layer.setLabeling() with null pointer.
 - ct is now a QgsCoordinateTransform object, not a pointer. An invalid QgsCoordinateTransform will
 be used instead of a null pointer if no transformation is required.
 - prepareGeometry() and geometryRequiresPreparation() now take a QgsCoordinateTransform reference, not a pointer. An invalid QgsCoordinateTransform should be used instead of a null pointer if no transformation is required.

diff --git a/python/core/qgspallabeling.sip b/python/core/qgspallabeling.sip
@@ -273,9 +273,6 @@ class QgsPalLayerSettings
      */
     static const QgsPropertiesDefinition &propertyDefinitions();
 
-    // whether to label this layer
-    bool enabled;
-
     /** Whether to draw labels for this layer. For some layers it may be desirable
      * to register their features as obstacles for other labels without requiring
      * labels to be drawn for the layer itself. In this case drawLabels can be set

diff --git a/src/app/dwg/qgsdwgimportdialog.cpp b/src/app/dwg/qgsdwgimportdialog.cpp
@@ -356,7 +356,6 @@ void QgsDwgImportDialog::createGroup( QgsLayerTreeGroup *group, QString name, QS
     QgsPalLayerSettings pls;
     pls.setFormat( tf );
 
-    pls.enabled = true;
     pls.drawLabels = true;
     pls.fieldName = "text";
     pls.wrapChar = "\\P";

diff --git a/src/app/qgslabelinggui.cpp b/src/app/qgslabelinggui.cpp
@@ -295,7 +295,6 @@ QgsPalLayerSettings QgsLabelingGui::layerSettings()
 {
   QgsPalLayerSettings lyr;
 
-  lyr.enabled = ( mMode == Labels || mMode == ObstaclesOnly );
   lyr.drawLabels = ( mMode == Labels );
 
   bool isExpression;

diff --git a/src/app/qgslabelingwidget.cpp b/src/app/qgslabelingwidget.cpp
@@ -90,15 +90,7 @@ void QgsLabelingWidget::adaptToLayer()
   {
     QgsPalLayerSettings lyr = mLayer->labeling()->settings();
 
-    // enable/disable main options based upon whether layer is being labeled
-    if ( !lyr.enabled )
-    {
-      mLabelModeComboBox->setCurrentIndex( 0 );
-    }
-    else
-    {
-      mLabelModeComboBox->setCurrentIndex( lyr.drawLabels ? 1 : 3 );
-    }
+    mLabelModeComboBox->setCurrentIndex( lyr.drawLabels ? 1 : 3 );
   }
   else
   {

diff --git a/src/app/qgsmaptoollabel.cpp b/src/app/qgsmaptoollabel.cpp
@@ -698,7 +698,7 @@ QgsMapToolLabel::LabelDetails::LabelDetails( const QgsLabelPosition &p )
     if ( p.isDiagram )
       valid = layer->diagramsEnabled();
     else
-      valid = settings.enabled;
+      valid = true;
   }
 
   if ( !valid )

diff --git a/src/core/qgspallabeling.cpp b/src/core/qgspallabeling.cpp
@@ -233,7 +233,6 @@ QgsPalLayerSettings::QgsPalLayerSettings()
 {
   initPropertyDefinitions();
 
-  enabled = false;
   drawLabels = true;
   isExpression = false;
   fieldIndex = 0;
@@ -317,7 +316,6 @@ QgsPalLayerSettings &QgsPalLayerSettings::operator=( const QgsPalLayerSettings &
 
   // copy only permanent stuff
 
-  enabled = s.enabled;
   drawLabels = s.drawLabels;
 
   // text style
@@ -534,7 +532,6 @@ void QgsPalLayerSettings::readFromLayerCustomProperties( QgsVectorLayer *layer )
 
   // NOTE: set defaults for newly added properties, for backwards compatibility
 
-  enabled = layer->labelsEnabled();
   drawLabels = layer->customProperty( QStringLiteral( "labeling/drawLabels" ), true ).toBool();
 
   mFormat.readFromLayer( layer );
@@ -673,9 +670,6 @@ void QgsPalLayerSettings::readFromLayerCustomProperties( QgsVectorLayer *layer )
 
 void QgsPalLayerSettings::readXml( QDomElement &elem, const QgsReadWriteContext &context )
 {
-  enabled = true;
-  drawLabels = true;
-
   // text style
   QDomElement textStyleElem = elem.firstChildElement( QStringLiteral( "text-style" ) );
   fieldName = textStyleElem.attribute( QStringLiteral( "fieldName" ) );
@@ -756,6 +750,9 @@ void QgsPalLayerSettings::readXml( QDomElement &elem, const QgsReadWriteContext
 
   // rendering
   QDomElement renderingElem = elem.firstChildElement( QStringLiteral( "rendering" ) );
+
+  drawLabels = renderingElem.attribute( QStringLiteral( "drawLabels" ), QStringLiteral( "1" ) ).toInt();
+
   scaleMin = renderingElem.attribute( QStringLiteral( "scaleMin" ), QStringLiteral( "0" ) ).toInt();
   scaleMax = renderingElem.attribute( QStringLiteral( "scaleMax" ), QStringLiteral( "0" ) ).toInt();
   scaleVisibility = renderingElem.attribute( QStringLiteral( "scaleVisibility" ) ).toInt();
@@ -794,7 +791,7 @@ void QgsPalLayerSettings::readXml( QDomElement &elem, const QgsReadWriteContext
 
 QDomElement QgsPalLayerSettings::writeXml( QDomDocument &doc, const QgsReadWriteContext &context )
 {
-  // we assume (enabled == true && drawLabels == true) so those are not saved
+
 
   QDomElement textStyleElem = mFormat.writeXml( doc, context );
 
@@ -848,6 +845,7 @@ QDomElement QgsPalLayerSettings::writeXml( QDomDocument &doc, const QgsReadWrite
 
   // rendering
   QDomElement renderingElem = doc.createElement( QStringLiteral( "rendering" ) );
+  renderingElem.setAttribute( QStringLiteral( "drawLabels" ), drawLabels );
   renderingElem.setAttribute( QStringLiteral( "scaleVisibility" ), scaleVisibility );
   renderingElem.setAttribute( QStringLiteral( "scaleMin" ), scaleMin );
   renderingElem.setAttribute( QStringLiteral( "scaleMax" ), scaleMax );

diff --git a/src/core/qgspallabeling.h b/src/core/qgspallabeling.h
@@ -377,10 +377,6 @@ class CORE_EXPORT QgsPalLayerSettings
      */
     static const QgsPropertiesDefinition &propertyDefinitions();
 
-
-    // whether to label this layer
-    bool enabled;
-
     /** Whether to draw labels for this layer. For some layers it may be desirable
      * to register their features as obstacles for other labels without requiring
      * labels to be drawn for the layer itself. In this case drawLabels can be set

diff --git a/src/core/qgsvectorlayer.cpp b/src/core/qgsvectorlayer.cpp
@@ -3513,8 +3513,7 @@ void QgsVectorLayer::readSldLabeling( const QDomNode &node )
     QDomElement propertyNameElem = labelElem.firstChildElement( QStringLiteral( "PropertyName" ) );
     if ( !propertyNameElem.isNull() )
     {
-      // enable labeling + set labeling defaults
-      settings.enabled = true;
+      // set labeling defaults
 
       // label attribute
       QString labelAttribute = propertyNameElem.text();

diff --git a/src/server/qgswmsconfigparser.cpp b/src/server/qgswmsconfigparser.cpp
@@ -389,7 +389,6 @@ QgsVectorLayer *QgsWmsConfigParser::createHighlightLayer( int i, const QString &
   }
 
   QgsPalLayerSettings settings;
-  settings.enabled = true;
   settings.fieldName = "label";
 
   //give highest priority to highlight layers and make sure the labels are always drawn

diff --git a/tests/src/core/testqgslabelingengine.cpp b/tests/src/core/testqgslabelingengine.cpp
@@ -98,8 +98,8 @@ void TestQgsLabelingEngine::cleanup()
 void TestQgsLabelingEngine::setDefaultLabelParams( QgsPalLayerSettings &settings )
 {
   QgsTextFormat format;
-  format.setFont( QgsFontUtils::getStandardTestFont( QStringLiteral( "Bold" ) ) );
-  format.setSize( 12.4 );    // TODO: why does it render nothing when point size == 12 ???
+  format.setFont( QgsFontUtils::getStandardTestFont( QStringLiteral( "Bold" ) ).family() );
+  format.setSize( 12 );
   format.setNamedStyle( "Bold" );
   format.setColor( QColor( 200, 0, 200 ) );
   settings.setFormat( format );
@@ -127,10 +127,11 @@ void TestQgsLabelingEngine::testBasic()
   context.setPainter( &p );
 
   QgsPalLayerSettings settings;
-  settings.enabled = true;
   settings.fieldName = "Class";
   setDefaultLabelParams( settings );
 
+  vl->setLabeling( new QgsVectorLayerSimpleLabeling( settings ) );  // TODO: this should not be necessary!
+
   QgsLabelingEngine engine;
   engine.setMapSettings( mapSettings );
   engine.addProvider( new QgsVectorLayerLabelProvider( vl, QString(), true, &settings ) );
@@ -211,7 +212,6 @@ void TestQgsLabelingEngine::testRuleBased()
   QgsRuleBasedLabeling::Rule *root = new QgsRuleBasedLabeling::Rule( 0 );
 
   QgsPalLayerSettings s1;
-  s1.enabled = true;
   s1.fieldName = QStringLiteral( "Class" );
   s1.obstacle = false;
   s1.dist = 2;
@@ -228,7 +228,6 @@ void TestQgsLabelingEngine::testRuleBased()
   root->appendChild( new QgsRuleBasedLabeling::Rule( new QgsPalLayerSettings( s1 ) ) );
 
   QgsPalLayerSettings s2;
-  s2.enabled = true;
   s2.fieldName = QStringLiteral( "Class" );
   s2.obstacle = false;
   s2.dist = 2;
@@ -303,7 +302,6 @@ void TestQgsLabelingEngine::zOrder()
   context.setPainter( &p );
 
   QgsPalLayerSettings pls1;
-  pls1.enabled = true;
   pls1.fieldName = QStringLiteral( "Class" );
   pls1.placement = QgsPalLayerSettings::OverPoint;
   pls1.quadOffset = QgsPalLayerSettings::QuadrantAboveRight;

diff --git a/tests/src/python/test_qgsmaprenderer.py b/tests/src/python/test_qgsmaprenderer.py
@@ -18,8 +18,11 @@
                        QgsMapRendererParallelJob,
                        QgsMapRendererSequentialJob,
                        QgsMapRendererCustomPainterJob,
+                       QgsPalLayerSettings,
                        QgsRectangle,
+                       QgsTextFormat,
                        QgsVectorLayer,
+                       QgsVectorLayerSimpleLabeling,
                        QgsFeature,
                        QgsGeometry,
                        QgsMapSettings,
@@ -123,9 +126,9 @@ def checkRepaintLabeledLayerInvalidatesLabelCache(self, job_type):
         layer = QgsVectorLayer("Point?field=fldtxt:string",
                                "layer1", "memory")
 
-        layer.setCustomProperty("labeling", "pal")
-        layer.setCustomProperty("labeling/enabled", True)
-        layer.setCustomProperty("labeling/fieldName", "fldtxt")
+        labelSettings = QgsPalLayerSettings()
+        labelSettings.fieldName = "fldtxt"
+        layer.setLabeling(QgsVectorLayerSimpleLabeling(labelSettings))
 
         settings = QgsMapSettings()
         settings.setExtent(QgsRectangle(5, 25, 25, 45))
@@ -164,9 +167,9 @@ def checkAddingNewLabeledLayerInvalidatesLabelCache(self, job_type):
         layer = QgsVectorLayer("Point?field=fldtxt:string",
                                "layer1", "memory")
 
-        layer.setCustomProperty("labeling", "pal")
-        layer.setCustomProperty("labeling/enabled", True)
-        layer.setCustomProperty("labeling/fieldName", "fldtxt")
+        labelSettings = QgsPalLayerSettings()
+        labelSettings.fieldName = "fldtxt"
+        layer.setLabeling(QgsVectorLayerSimpleLabeling(labelSettings))
 
         settings = QgsMapSettings()
         settings.setExtent(QgsRectangle(5, 25, 25, 45))
@@ -188,9 +191,7 @@ def checkAddingNewLabeledLayerInvalidatesLabelCache(self, job_type):
         # add another labeled layer
         layer2 = QgsVectorLayer("Point?field=fldtxt:string",
                                 "layer2", "memory")
-        layer2.setCustomProperty("labeling", "pal")
-        layer2.setCustomProperty("labeling/enabled", True)
-        layer2.setCustomProperty("labeling/fieldName", "fldtxt")
+        layer2.setLabeling(QgsVectorLayerSimpleLabeling(labelSettings))
         settings.setLayers([layer, layer2])
 
         # second job should not be able to use label cache, since a new layer was added
@@ -210,9 +211,9 @@ def checkAddingNewNonLabeledLayerKeepsLabelCache(self, job_type):
         layer = QgsVectorLayer("Point?field=fldtxt:string",
                                "layer1", "memory")
 
-        layer.setCustomProperty("labeling", "pal")
-        layer.setCustomProperty("labeling/enabled", True)
-        layer.setCustomProperty("labeling/fieldName", "fldtxt")
+        labelSettings = QgsPalLayerSettings()
+        labelSettings.fieldName = "fldtxt"
+        layer.setLabeling(QgsVectorLayerSimpleLabeling(labelSettings))
 
         settings = QgsMapSettings()
         settings.setExtent(QgsRectangle(5, 25, 25, 45))
@@ -253,15 +254,13 @@ def checkRemovingLabeledLayerInvalidatesLabelCache(self, job_type):
         layer = QgsVectorLayer("Point?field=fldtxt:string",
                                "layer1", "memory")
 
-        layer.setCustomProperty("labeling", "pal")
-        layer.setCustomProperty("labeling/enabled", True)
-        layer.setCustomProperty("labeling/fieldName", "fldtxt")
+        labelSettings = QgsPalLayerSettings()
+        labelSettings.fieldName = "fldtxt"
+        layer.setLabeling(QgsVectorLayerSimpleLabeling(labelSettings))
 
         layer2 = QgsVectorLayer("Point?field=fldtxt:string",
                                 "layer2", "memory")
-        layer2.setCustomProperty("labeling", "pal")
-        layer2.setCustomProperty("labeling/enabled", True)
-        layer2.setCustomProperty("labeling/fieldName", "fldtxt")
+        layer2.setLabeling(QgsVectorLayerSimpleLabeling(labelSettings))
 
         settings = QgsMapSettings()
         settings.setExtent(QgsRectangle(5, 25, 25, 45))
@@ -300,9 +299,9 @@ def checkRemovingNonLabeledLayerKeepsLabelCache(self, job_type):
         layer = QgsVectorLayer("Point?field=fldtxt:string",
                                "layer1", "memory")
 
-        layer.setCustomProperty("labeling", "pal")
-        layer.setCustomProperty("labeling/enabled", True)
-        layer.setCustomProperty("labeling/fieldName", "fldtxt")
+        labelSettings = QgsPalLayerSettings()
+        labelSettings.fieldName = "fldtxt"
+        layer.setLabeling(QgsVectorLayerSimpleLabeling(labelSettings))
 
         layer2 = QgsVectorLayer("Point?field=fldtxt:string",
                                 "layer2", "memory")
@@ -344,16 +343,18 @@ def checkLabeledLayerWithBlendModesCannotBeCached(self, job_type):
         layer = QgsVectorLayer("Point?field=fldtxt:string",
                                "layer1", "memory")
 
-        layer.setCustomProperty("labeling", "pal")
-        layer.setCustomProperty("labeling/enabled", True)
-        layer.setCustomProperty("labeling/fieldName", "fldtxt")
+        labelSettings = QgsPalLayerSettings()
+        labelSettings.fieldName = "fldtxt"
+        layer.setLabeling(QgsVectorLayerSimpleLabeling(labelSettings))
 
         layer2 = QgsVectorLayer("Point?field=fldtxt:string",
                                 "layer2", "memory")
-        layer2.setCustomProperty("labeling", "pal")
-        layer2.setCustomProperty("labeling/enabled", True)
-        layer2.setCustomProperty("labeling/fieldName", "fldtxt")
-        layer2.setCustomProperty("labeling/blendMode", 5)
+        labelSettings2 = QgsPalLayerSettings()
+        labelSettings2.fieldName = "fldtxt"
+        format2 = QgsTextFormat()
+        format2.setBlendMode(QPainter.CompositionMode_SourceIn)
+        labelSettings2.setFormat(format2)
+        layer2.setLabeling(QgsVectorLayerSimpleLabeling(labelSettings2))
 
         settings = QgsMapSettings()
         settings.setExtent(QgsRectangle(5, 25, 25, 45))

diff --git a/tests/src/python/test_qgspallabeling_base.py b/tests/src/python/test_qgspallabeling_base.py
@@ -251,7 +251,6 @@ def configTest(self, prefix, abbr):
 
     def defaultLayerSettings(self):
         lyr = QgsPalLayerSettings()
-        lyr.enabled = True
         lyr.fieldName = 'text'  # default in test data sources
         font = self.getTestFont()
         font.setPointSize(32)

diff --git a/tests/src/python/test_qgspallabeling_placement.py b/tests/src/python/test_qgspallabeling_placement.py
@@ -66,7 +66,8 @@ def setUp(self):
         self._MapSettings.setLabelingEngineSettings(engine_settings)
 
     def checkTest(self, **kwargs):
-        self.layer.setLabeling(QgsVectorLayerSimpleLabeling(self.lyr))
+        if kwargs.get('apply_simple_labeling', True):
+            self.layer.setLabeling(QgsVectorLayerSimpleLabeling(self.lyr))
 
         ms = self._MapSettings  # class settings
         settings_type = 'Class'
@@ -172,8 +173,7 @@ def test_polygon_multiple_labels(self):
         # is INSIDE the polygon
         self.layer = TestQgsPalLabeling.loadFeatureLayer('polygon_rule_based')
         self._TestMapSettings = self.cloneMapSettings(self._MapSettings)
-        self.lyr.placement = QgsPalLayerSettings.Horizontal
-        self.checkTest()
+        self.checkTest(apply_simple_labeling=False)
         self.removeMapLayer(self.layer)
         self.layer = None