From f153e19168d86b60d49e898efe9617f1505a725d Mon Sep 17 00:00:00 2001 From: Martin Dobias Date: Tue, 16 May 2017 01:39:00 +0800 Subject: [PATCH] Fix saving of "obstacle only" state + misc fixes to unit tests --- doc/api_break.dox | 1 + python/core/qgspallabeling.sip | 3 - src/app/dwg/qgsdwgimportdialog.cpp | 1 - src/app/qgslabelinggui.cpp | 1 - src/app/qgslabelingwidget.cpp | 10 +--- src/app/qgsmaptoollabel.cpp | 2 +- src/core/qgspallabeling.cpp | 12 ++-- src/core/qgspallabeling.h | 4 -- src/core/qgsvectorlayer.cpp | 3 +- src/server/qgswmsconfigparser.cpp | 1 - tests/src/core/testqgslabelingengine.cpp | 10 ++-- tests/src/python/test_qgsmaprenderer.py | 57 ++++++++++--------- tests/src/python/test_qgspallabeling_base.py | 1 - .../python/test_qgspallabeling_placement.py | 6 +- 14 files changed, 45 insertions(+), 67 deletions(-) diff --git a/doc/api_break.dox b/doc/api_break.dox index 85c7193b291..4f2780d5d7f 100644 --- 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 index eac9be72a7f..ff2823cff15 100644 --- 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 index af734919985..8178a2e4466 100644 --- 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 index 7c97d0694a7..a27b7e32a00 100644 --- 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 index c18f35dd1ce..dacf7199e01 100644 --- 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 index 502c4c8949d..a7a940574d6 100644 --- 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 index 639fa3548bd..f6a2f4fe9ed 100644 --- 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 index 0d5a085abe5..1c4b95de572 100644 --- 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 index eea3e3550a8..19d04fb810e 100644 --- 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 index 89e1c14960f..a06f9a1531d 100644 --- 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 index be8598c59b2..5dffec06403 100644 --- 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 index d63fcfb6613..72ef1343b50 100644 --- a/tests/src/python/test_qgsmaprenderer.py +++ b/tests/src/python/test_qgsmaprenderer.py @@ -18,8 +18,11 @@ from qgis.core import (QgsMapRendererCache, QgsMapRendererParallelJob, QgsMapRendererSequentialJob, QgsMapRendererCustomPainterJob, + QgsPalLayerSettings, QgsRectangle, + QgsTextFormat, QgsVectorLayer, + QgsVectorLayerSimpleLabeling, QgsFeature, QgsGeometry, QgsMapSettings, @@ -123,9 +126,9 @@ class TestQgsMapRenderer(unittest.TestCase): 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 @@ class TestQgsMapRenderer(unittest.TestCase): 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 @@ class TestQgsMapRenderer(unittest.TestCase): # 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 @@ class TestQgsMapRenderer(unittest.TestCase): 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 @@ class TestQgsMapRenderer(unittest.TestCase): 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 @@ class TestQgsMapRenderer(unittest.TestCase): 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 @@ class TestQgsMapRenderer(unittest.TestCase): 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 index 929fb94d779..c7b7a9830c7 100644 --- a/tests/src/python/test_qgspallabeling_base.py +++ b/tests/src/python/test_qgspallabeling_base.py @@ -251,7 +251,6 @@ class TestQgsPalLabeling(unittest.TestCase): 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 index 44e3dd1112c..dee4c3a4e78 100644 --- a/tests/src/python/test_qgspallabeling_placement.py +++ b/tests/src/python/test_qgspallabeling_placement.py @@ -66,7 +66,8 @@ class TestPlacementBase(TestQgsPalLabeling): 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 @@ class TestPointPlacement(TestPlacementBase): # 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