From 1c756348680f44ccbfcc612d3314ef615de73855 Mon Sep 17 00:00:00 2001
From: codereader <greebo@angua.at>
Date: Sat, 30 Oct 2021 08:08:53 +0200
Subject: [PATCH] #5746: Improve texture tool scene analysis code

---
 .../textool/TextureToolSceneGraph.cpp         | 36 +++++++++++-------
 .../selection/textool/TextureToolSceneGraph.h |  2 +-
 test/TextureTool.cpp                          | 37 +++++++++++++++++++
 3 files changed, 60 insertions(+), 15 deletions(-)

diff --git a/radiantcore/selection/textool/TextureToolSceneGraph.cpp b/radiantcore/selection/textool/TextureToolSceneGraph.cpp
index 3663c456fe..76b5b62523 100644
--- a/radiantcore/selection/textool/TextureToolSceneGraph.cpp
+++ b/radiantcore/selection/textool/TextureToolSceneGraph.cpp
@@ -13,7 +13,8 @@ namespace textool
 {
 
 TextureToolSceneGraph::TextureToolSceneGraph() :
-    _selectionNeedsRescan(true)
+    _selectionNeedsRescan(true),
+    _activeMaterialNeedsRescan(true)
 {}
 
 const std::string& TextureToolSceneGraph::getName() const
@@ -44,10 +45,11 @@ void TextureToolSceneGraph::initialiseModule(const IApplicationContext& ctx)
 
 void TextureToolSceneGraph::shutdownModule()
 {
-    GlobalRadiantCore().getMessageBus().removeListener(_textureChangedHandler);
     _selectionNeedsRescan = false;
+    _activeMaterialNeedsRescan = false;
     _nodes.clear();
     _sceneSelectionChanged.disconnect();
+    GlobalRadiantCore().getMessageBus().removeListener(_textureChangedHandler);
 }
 
 void TextureToolSceneGraph::foreachNode(const std::function<bool(const INode::Ptr&)>& functor)
@@ -66,29 +68,34 @@ void TextureToolSceneGraph::foreachNode(const std::function<bool(const INode::Pt
 const std::string& TextureToolSceneGraph::getActiveMaterial()
 {
     ensureSceneIsAnalysed();
-    ensureActiveMaterialIsAnalysed();
 
     return _activeMaterial;
 }
 
-void TextureToolSceneGraph::ensureActiveMaterialIsAnalysed()
+void TextureToolSceneGraph::ensureSceneIsAnalysed()
 {
-    if (!_activeMaterialNeedsRescan) return;
+    // First, check if the material has changed
+    if (_activeMaterialNeedsRescan)
+    {
+        _activeMaterialNeedsRescan = false;
 
-    _activeMaterialNeedsRescan = false;
-    _activeMaterial = selection::getShaderFromSelection();
-}
+        auto material = selection::getShaderFromSelection();
+
+        if (material != _activeMaterial)
+        {
+            _activeMaterial = std::move(material);
+
+            // The material changed, we need to rebuild the nodes
+            _selectionNeedsRescan = true;
+        }
+    }
 
-void TextureToolSceneGraph::ensureSceneIsAnalysed()
-{
     if (!_selectionNeedsRescan) return;
     
     _selectionNeedsRescan = false;
-    _activeMaterialNeedsRescan = false;
-
     _nodes.clear();
 
-    _activeMaterial = selection::getShaderFromSelection();
+    // No unique material, leave everything empty
     if (_activeMaterial.empty()) return;
 
     if (GlobalSelectionSystem().countSelectedComponents() > 0)
@@ -99,7 +106,7 @@ void TextureToolSceneGraph::ensureSceneIsAnalysed()
             _nodes.emplace_back(std::make_shared<FaceNode>(face));
         });
     }
-    
+
     GlobalSelectionSystem().foreachSelected([&](const scene::INodePtr& node)
     {
         if (Node_isBrush(node))
@@ -122,6 +129,7 @@ void TextureToolSceneGraph::ensureSceneIsAnalysed()
 void TextureToolSceneGraph::onSceneSelectionChanged(const ISelectable& selectable)
 {
     // Mark our own selection as dirty
+    _activeMaterialNeedsRescan = true;
     _selectionNeedsRescan = true;
 }
 
diff --git a/radiantcore/selection/textool/TextureToolSceneGraph.h b/radiantcore/selection/textool/TextureToolSceneGraph.h
index 83beee1fd7..8a7040d020 100644
--- a/radiantcore/selection/textool/TextureToolSceneGraph.h
+++ b/radiantcore/selection/textool/TextureToolSceneGraph.h
@@ -38,8 +38,8 @@ class TextureToolSceneGraph :
 private:
     void onSceneSelectionChanged(const ISelectable& selectable);
     void onTextureChanged(radiant::TextureChangedMessage& msg);
+
     void ensureSceneIsAnalysed();
-    void ensureActiveMaterialIsAnalysed();
 };
 
 }
diff --git a/test/TextureTool.cpp b/test/TextureTool.cpp
index 281226c1b5..c74dd98a7e 100644
--- a/test/TextureTool.cpp
+++ b/test/TextureTool.cpp
@@ -205,6 +205,43 @@ TEST_F(TextureToolTest, SceneGraphRecognisesPatches)
     EXPECT_GT(getTextureToolNodeCount(), 0) << "There should be some tex tool nodes now";
 }
 
+// Selecting an inhomogenously textured brush will result in an empty scene graph
+TEST_F(TextureToolTest, SceneGraphSkipsInhomogeneousBrushes)
+{
+    auto worldspawn = GlobalMapModule().findOrInsertWorldspawn();
+    auto brush1 = algorithm::createCubicBrush(worldspawn, Vector3(0, 0, 0), "textures/numbers/1");
+    Node_getIBrush(brush1)->getFace(2).setShader("textures/numbers/2");
+
+    Node_setSelected(brush1, true);
+    EXPECT_EQ(GlobalSelectionSystem().countSelected(), 1) << "1 Brush must be selected";
+
+    // No tex tool nodes should show up here
+    EXPECT_EQ(getTextureToolNodeCount(), 0) << "There shouldn't be any tex tool nodes here";
+}
+
+// Harmonising the textures of all brush faces should make it show up in the texture tool
+TEST_F(TextureToolTest, SceneGraphUpdatesOnBrushMaterialChange)
+{
+    auto worldspawn = GlobalMapModule().findOrInsertWorldspawn();
+    auto brush1Node = algorithm::createCubicBrush(worldspawn, Vector3(0, 0, 0), "textures/numbers/1");
+    auto brush1 = Node_getIBrush(brush1Node);
+    brush1->getFace(2).setShader("textures/numbers/2");
+
+    Node_setSelected(brush1Node, true);
+    EXPECT_EQ(GlobalSelectionSystem().countSelected(), 1) << "1 Brush must be selected";
+
+    // No tex tool nodes should show up here
+    EXPECT_EQ(getTextureToolNodeCount(), 0) << "There shouldn't be any tex tool nodes here";
+
+    // Now apply the same texture to all faces
+    for (int i = 0; i < brush1->getNumFaces(); ++i)
+    {
+        brush1->getFace(i).setShader("textures/numbers/1");
+    }
+
+    EXPECT_EQ(getTextureToolNodeCount(), brush1->getNumFaces()) << "There shoud be 6 faces in the tex tool";
+}
+
 TEST_F(TextureToolTest, PatchNodeBounds)
 {
     auto worldspawn = GlobalMapModule().findOrInsertWorldspawn();