#5746: Improve texture tool scene analysis code

codereader · Oct 30, 2021 · 1c75634 · 1c75634
1 parent 77ebadd
commit 1c75634
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 15 deletions.
diff --git 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
@@ -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
@@ -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();