#6107: Implement recursive behaviour of setLayerVisibility

codereader · Oct 1, 2022 · 87df89b · 87df89b
1 parent b6fa091
commit 87df89b
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 22 deletions.
diff --git a/radiantcore/layers/LayerManager.cpp b/radiantcore/layers/LayerManager.cpp
@@ -220,24 +220,11 @@ bool LayerManager::layerIsVisible(int layerID)
 	return _layerVisibility[layerID];
 }
 
-void LayerManager::setLayerVisibility(int layerID, bool visible)
+void LayerManager::setLayerVisibility(int layerId, bool visible)
 {
-	// Sanity check
-	if (layerID < 0 || layerID >= static_cast<int>(_layerVisibility.size()))
-	{
-		rMessage() << "LayerSystem: Setting visibility of invalid layer ID: " << layerID << std::endl;
-		return;
-	}
+    auto layerVisibilityChanged = setLayerVisibilityRecursively(layerId, visible);
 
-    if (_layerVisibility[layerID] == visible)
-    {
-        return; // nothing to change here
-    }
-
-	// Set the visibility
-	_layerVisibility[layerID] = visible;
-
-	if (!visible && layerID == _activeLayer)
+	if (!visible && !_layerVisibility.at(_activeLayer))
 	{
 		// We just hid the active layer, fall back to another one
 		_activeLayer = getFirstVisibleLayer();
@@ -248,11 +235,40 @@ void LayerManager::setLayerVisibility(int layerID, bool visible)
     if (visible && _activeLayer < static_cast<int>(_layerVisibility.size()) && 
         !_layerVisibility[_activeLayer])
     {
-        _activeLayer = layerID;
+        _activeLayer = layerId;
+    }
+
+    if (layerVisibilityChanged)
+    {
+	    // Fire the visibility changed event
+	    onLayerVisibilityChanged();
+    }
+}
+
+bool LayerManager::setLayerVisibilityRecursively(int layerId, bool visible)
+{
+    if (layerId < 0 || layerId >= static_cast<int>(_layerVisibility.size()))
+    {
+        rMessage() << "LayerSystem: Setting visibility of invalid layer ID: " << layerId << std::endl;
+        return false;
+    }
+
+    bool visibilityChange = _layerVisibility.at(layerId) != visible;
+
+    _layerVisibility.at(layerId) = visible;
+
+    // Set the visibility of all child layers too
+    // Start with index 1, as the default layer cannot have any parent
+    for (std::size_t childLayerId = 1; childLayerId < _layerParentIds.size(); ++childLayerId)
+    {
+        // If this local layer is assigned as parent to this child layer, recurse into it
+        if (_layerParentIds.at(childLayerId) == layerId)
+        {
+            visibilityChange |= setLayerVisibilityRecursively(static_cast<int>(childLayerId), visible);
+        }
     }
 
-	// Fire the visibility changed event
-	onLayerVisibilityChanged();
+    return visibilityChange;
 }
 
 void LayerManager::updateSceneGraphVisibility()

diff --git a/radiantcore/layers/LayerManager.h b/radiantcore/layers/LayerManager.h
@@ -120,6 +120,11 @@ class LayerManager :
 	sigc::signal<void> signal_nodeMembershipChanged() override;
 
 private:
+    // Recursively sets the visibility of the given layer and updates
+    // the flags on the _layerVisibility vector.
+    // Returns true if any flag changed, false if nothing changed.
+    bool setLayerVisibilityRecursively(int layerID, bool visible);
+
 	// Internal event emitter
 	void onLayersChanged();
 

diff --git a/test/LayerManipulation.cpp b/test/LayerManipulation.cpp
@@ -425,13 +425,15 @@ TEST_F(LayerTest, SetLayerVisibilityWorksRecursively)
 
     auto& layerManager = GlobalMapModule().getRoot()->getLayerManager();
     auto testLayer2Id = layerManager.getLayerID("Test2");
+    auto testLayer7Id = layerManager.getLayerID("Test7");
     auto boardsLayerId = layerManager.getLayerID("BoardsandStuff");
     EXPECT_EQ(layerManager.getParentLayer(testLayer2Id), boardsLayerId) << "Test setup is wrong";
+    EXPECT_EQ(layerManager.getParentLayer(testLayer7Id), testLayer2Id) << "Test setup is wrong";
 
-    // Move the worldspawn to the child layer
+    // Move the worldspawn to the grand-child layer
     auto worldspawn = GlobalMapModule().findOrInsertWorldspawn();
     Node_setSelected(worldspawn, true);
-    layerManager.moveSelectionToLayer(testLayer2Id);
+    layerManager.moveSelectionToLayer(testLayer7Id);
     Node_setSelected(worldspawn, false);
 
     EXPECT_TRUE(worldspawn->visible());
@@ -440,12 +442,16 @@ TEST_F(LayerTest, SetLayerVisibilityWorksRecursively)
     layerManager.setLayerVisibility(boardsLayerId, false);
     EXPECT_FALSE(worldspawn->visible()) << "Worldspawn should be hidden now";
 
-    EXPECT_FALSE(layerManager.layerIsVisible(testLayer2Id)) << 
+    EXPECT_FALSE(layerManager.layerIsVisible(testLayer7Id)) <<
+        "The parent layer visibility should have propagated down to the boards layer";
+    EXPECT_FALSE(layerManager.layerIsVisible(testLayer2Id)) <<
         "The parent layer visibility should have propagated down to the boards layer";
 
     layerManager.setLayerVisibility(boardsLayerId, true);
     EXPECT_TRUE(worldspawn->visible()) << "Worldspawn should be visible again";
 
+    EXPECT_TRUE(layerManager.layerIsVisible(testLayer7Id)) <<
+        "The parent layer visibility should have propagated down to the boards layer";
     EXPECT_TRUE(layerManager.layerIsVisible(testLayer2Id)) <<
         "The parent layer visibility should have propagated down to the boards layer";
 }