[layouts] Fix layers are incorrectly shown when groups are unchecked

for a map theme Followup #33099 Fixes #34257 (cherry picked from commit 38a3a2d) (cherry picked from commit 5a8e131)
qgis · Mar 17, 2020 · b7bc1c3 · b7bc1c3
1 parent ee5e99b
commit b7bc1c3
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 8 deletions.
diff --git a/python/core/auto_generated/qgsmapthemecollection.sip.in b/python/core/auto_generated/qgsmapthemecollection.sip.in
@@ -55,6 +55,8 @@ Returns map layer or ``None`` if the layer does not exist anymore
 Sets the map layer for this record
 %End
 
+        bool isVisible;
+
         bool usingCurrentStyle;
         QString currentStyle;
         bool usingLegendItems;

diff --git a/src/core/qgsmapthemecollection.cpp b/src/core/qgsmapthemecollection.cpp
@@ -35,6 +35,7 @@ QgsMapThemeCollection::QgsMapThemeCollection( QgsProject *project )
 QgsMapThemeCollection::MapThemeLayerRecord QgsMapThemeCollection::createThemeLayerRecord( QgsLayerTreeLayer *nodeLayer, QgsLayerTreeModel *model )
 {
   MapThemeLayerRecord layerRec( nodeLayer->layer() );
+  layerRec.isVisible = nodeLayer->isVisible();
   layerRec.usingCurrentStyle = true;
   layerRec.currentStyle = nodeLayer->layer()->styleManager()->currentStyle();
   layerRec.expandedLayerNode = nodeLayer->isExpanded();
@@ -124,15 +125,20 @@ bool QgsMapThemeCollection::findRecordForLayer( QgsMapLayer *layer, const QgsMap
 void QgsMapThemeCollection::applyThemeToLayer( QgsLayerTreeLayer *nodeLayer, QgsLayerTreeModel *model, const QgsMapThemeCollection::MapThemeRecord &rec )
 {
   MapThemeLayerRecord layerRec;
-  bool isVisible = findRecordForLayer( nodeLayer->layer(), rec, layerRec );
+  const bool recordExists = findRecordForLayer( nodeLayer->layer(), rec, layerRec );
 
   // Make sure the whole tree is visible
-  if ( isVisible )
-    nodeLayer->setItemVisibilityCheckedParentRecursive( isVisible );
+  if ( recordExists )
+  {
+    if ( rec.hasCheckedStateInfo() )
+      nodeLayer->setItemVisibilityChecked( true );
+    else
+      nodeLayer->setItemVisibilityCheckedParentRecursive( true );
+  }
   else
-    nodeLayer->setItemVisibilityChecked( isVisible );
+    nodeLayer->setItemVisibilityChecked( false );
 
-  if ( !isVisible )
+  if ( !recordExists )
     return;
 
   if ( layerRec.usingCurrentStyle )
@@ -322,7 +328,7 @@ QList<QgsMapLayer *> QgsMapThemeCollection::mapThemeVisibleLayers( const QString
     const auto records {mMapThemes.value( name ).mLayerRecords};
     for ( const MapThemeLayerRecord &layerRec : records )
     {
-      if ( layerRec.layer() )
+      if ( layerRec.isVisible && layerRec.layer() )
         layers << layerRec.layer();
     }
   }
@@ -334,7 +340,7 @@ QList<QgsMapLayer *> QgsMapThemeCollection::mapThemeVisibleLayers( const QString
       const auto constRecs = recs;
       for ( const MapThemeLayerRecord &layerRec : constRecs )
       {
-        if ( layerRec.layer() == layer )
+        if ( layerRec.isVisible && layerRec.layer() == layer )
           layers << layerRec.layer();
       }
     }
@@ -448,6 +454,7 @@ void QgsMapThemeCollection::readXml( const QDomDocument &doc )
       if ( QgsMapLayer *layer = mProject->mapLayer( layerID ) )
       {
         layerRecords[layerID] = MapThemeLayerRecord( layer );
+        layerRecords[layerID].isVisible = visPresetLayerElem.attribute( QStringLiteral( "visible" ), QStringLiteral( "1" ) ).toInt();
 
         if ( visPresetLayerElem.hasAttribute( QStringLiteral( "style" ) ) )
         {
@@ -575,6 +582,7 @@ void QgsMapThemeCollection::writeXml( QDomDocument &doc )
       QString layerID = layerRec.layer()->id();
       QDomElement layerElem = doc.createElement( QStringLiteral( "layer" ) );
       layerElem.setAttribute( QStringLiteral( "id" ), layerID );
+      layerElem.setAttribute( QStringLiteral( "visible" ), layerRec.isVisible ? QStringLiteral( "1" ) : QStringLiteral( "0" ) );
       if ( layerRec.usingCurrentStyle )
         layerElem.setAttribute( QStringLiteral( "style" ), layerRec.currentStyle );
       visPresetElem.appendChild( layerElem );

diff --git a/src/core/qgsmapthemecollection.h b/src/core/qgsmapthemecollection.h
@@ -63,7 +63,7 @@ class CORE_EXPORT QgsMapThemeCollection : public QObject
 
         bool operator==( const QgsMapThemeCollection::MapThemeLayerRecord &other ) const
         {
-          return mLayer == other.mLayer &&
+          return mLayer == other.mLayer && isVisible == other.isVisible &&
                  usingCurrentStyle == other.usingCurrentStyle && currentStyle == other.currentStyle &&
                  usingLegendItems == other.usingLegendItems && checkedLegendItems == other.checkedLegendItems &&
                  expandedLegendItems == other.expandedLegendItems && expandedLayerNode == other.expandedLayerNode;
@@ -79,6 +79,12 @@ class CORE_EXPORT QgsMapThemeCollection : public QObject
         //! Sets the map layer for this record
         void setLayer( QgsMapLayer *layer );
 
+        /**
+         * TRUE if the layer is visible in the associated theme.
+         * \since QGIS 3.14
+         */
+        bool isVisible = true;
+
         //! Whether current style is valid and should be applied
         bool usingCurrentStyle = false;
         //! Name of the current style of the layer

diff --git a/tests/src/core/testqgsmapthemecollection.cpp b/tests/src/core/testqgsmapthemecollection.cpp
@@ -244,12 +244,21 @@ void TestQgsMapThemeCollection::checkedState()
   QgsMapThemeCollection::MapThemeRecord recUnchecked = themes.mapThemeState( "all-unchecked" );
   QVERIFY( recUnchecked.hasCheckedStateInfo() );
   QCOMPARE( recUnchecked.checkedGroupNodes().count(), 0 );
+  QVERIFY( themes.mapThemeVisibleLayers( QStringLiteral( "all-unchecked" ) ).isEmpty() );
+  QVERIFY( themes.mapThemeVisibleLayerIds( QStringLiteral( "all-unchecked" ) ).isEmpty() );
   QCOMPARE( mNodeLayerLines->itemVisibilityChecked(), false );
   QCOMPARE( mNodeLayerPolys->itemVisibilityChecked(), true );
 
   QgsMapThemeCollection::MapThemeRecord recChecked = themes.mapThemeState( "all-checked" );
   QVERIFY( recChecked.hasCheckedStateInfo() );
   QCOMPARE( recChecked.checkedGroupNodes().count(), 3 );
+  QCOMPARE( themes.mapThemeVisibleLayers( QStringLiteral( "all-checked" ) ).size(), 2 );
+  QVERIFY( themes.mapThemeVisibleLayers( QStringLiteral( "all-checked" ) ).contains( mPolysLayer ) );
+  QVERIFY( themes.mapThemeVisibleLayers( QStringLiteral( "all-checked" ) ).contains( mPointsLayer ) );
+  QCOMPARE( themes.mapThemeVisibleLayerIds( QStringLiteral( "all-checked" ) ).size(), 2 );
+  QVERIFY( themes.mapThemeVisibleLayerIds( QStringLiteral( "all-checked" ) ).contains( mPolysLayer->id() ) );
+  QVERIFY( themes.mapThemeVisibleLayerIds( QStringLiteral( "all-checked" ) ).contains( mPointsLayer->id() ) );
+
   QCOMPARE( mNodeLayerLines->itemVisibilityChecked(), false );
   QCOMPARE( mNodeLayerPolys->itemVisibilityChecked(), true );
 
@@ -280,10 +289,18 @@ void TestQgsMapThemeCollection::checkedState()
   QgsMapThemeCollection::MapThemeRecord recUnchecked2 = themes2.mapThemeState( "all-unchecked" );
   QVERIFY( recUnchecked2.hasCheckedStateInfo() );
   QCOMPARE( recUnchecked2.checkedGroupNodes().count(), 0 );
+  QVERIFY( themes2.mapThemeVisibleLayers( QStringLiteral( "all-unchecked" ) ).isEmpty() );
+  QVERIFY( themes2.mapThemeVisibleLayerIds( QStringLiteral( "all-unchecked" ) ).isEmpty() );
 
   QgsMapThemeCollection::MapThemeRecord recChecked2 = themes2.mapThemeState( "all-checked" );
   QVERIFY( recChecked2.hasCheckedStateInfo() );
   QCOMPARE( recChecked2.checkedGroupNodes().count(), 3 );
+  QCOMPARE( themes2.mapThemeVisibleLayers( QStringLiteral( "all-checked" ) ).size(), 2 );
+  QVERIFY( themes2.mapThemeVisibleLayers( QStringLiteral( "all-checked" ) ).contains( mPolysLayer ) );
+  QVERIFY( themes2.mapThemeVisibleLayers( QStringLiteral( "all-checked" ) ).contains( mPointsLayer ) );
+  QCOMPARE( themes2.mapThemeVisibleLayerIds( QStringLiteral( "all-checked" ) ).size(), 2 );
+  QVERIFY( themes2.mapThemeVisibleLayerIds( QStringLiteral( "all-checked" ) ).contains( mPolysLayer->id() ) );
+  QVERIFY( themes2.mapThemeVisibleLayerIds( QStringLiteral( "all-checked" ) ).contains( mPointsLayer->id() ) );
 }
 
 QGSTEST_MAIN( TestQgsMapThemeCollection )