#5746: Add selection changed feedback loop to detect programmatic cha…

…nges to ISelectable nodes

include/itexturetoolmodel.h

@@ -163,6 +163,8 @@ class ITextureToolSelectionSystem :
     virtual std::size_t countSelected() = 0;
     virtual std::size_t countSelectedComponentNodes() = 0;
 
+    virtual sigc::signal<void>& signal_selectionChanged() = 0;
+
     virtual void clearSelection() = 0;
     virtual void clearComponentSelection() = 0;
 
@@ -191,6 +193,9 @@ class ITextureToolSelectionSystem :
     virtual void onManipulationChanged() = 0;
     virtual void onManipulationFinished() = 0;
     virtual void onManipulationCancelled() = 0;
+
+    // Feedback method which is invoked by selectable nodes to report a selection status change
+    virtual void onNodeSelectionChanged(ISelectable& selectable) = 0;
 };
 
 }

radiantcore/selection/textool/NodeBase.h

@@ -2,23 +2,30 @@
 
 #include <vector>
 #include "itexturetoolmodel.h"
-#include "../BasicSelectable.h"
+#include <sigc++/functors/mem_fun.h>
+#include "itexturetoolmodel.h"
+#include "ObservedSelectable.h"
 #include "SelectableVertex.h"
 
 namespace textool
 {
 
 class NodeBase :
     public virtual INode,
-    public virtual IComponentSelectable
+    public virtual IComponentSelectable,
+    public std::enable_shared_from_this<NodeBase>
 {
 private:
-    selection::BasicSelectable _selectable;
+    selection::ObservedSelectable _selectable;
 
 protected:
     std::vector<SelectableVertex> _vertices;
 
 public:
+    NodeBase() :
+        _selectable(sigc::mem_fun(*this, &NodeBase::onSelectionStatusChanged))
+    {}
+
     virtual void setSelected(bool select) override
     {
         _selectable.setSelected(select);
@@ -106,6 +113,12 @@ class NodeBase :
         glEnd();
         glDisable(GL_DEPTH_TEST);
     }
+
+private:
+    void onSelectionStatusChanged(const ISelectable& selectable)
+    {
+        GlobalTextureToolSelectionSystem().onNodeSelectionChanged(*this);
+    }
 };
 
 }

radiantcore/selection/textool/TextureToolSelectionSystem.cpp

@@ -52,8 +52,12 @@ void TextureToolSelectionSystem::initialiseModule(const IApplicationContext& ctx
 
 void TextureToolSelectionSystem::shutdownModule()
 {
+    clearComponentSelection();
+    clearSelection();
+
     GlobalRadiantCore().getMessageBus().removeListener(_unselectListener);
 
+    _sigSelectionChanged.clear();
     _sigSelectionModeChanged.clear();
     _sigActiveManipulatorChanged.clear();
     _manipulators.clear();
@@ -208,6 +212,11 @@ std::size_t TextureToolSelectionSystem::countSelectedComponentNodes()
     return count;
 }
 
+sigc::signal<void>& TextureToolSelectionSystem::signal_selectionChanged()
+{
+    return _sigSelectionChanged;
+}
+
 void TextureToolSelectionSystem::clearSelection()
 {
     foreachSelectedNode([&](const INode::Ptr& node)
@@ -508,6 +517,11 @@ void TextureToolSelectionSystem::performSelectionTest(Selector& selector, Select
     });
 }
 
+void TextureToolSelectionSystem::onNodeSelectionChanged(ISelectable& selectable)
+{
+    _sigSelectionChanged.emit();
+}
+
 module::StaticModule<TextureToolSelectionSystem> _textureToolSelectionSystemModule;
 
 }

radiantcore/selection/textool/TextureToolSelectionSystem.h

@@ -1,5 +1,6 @@
 #pragma once
 
+#include <functional>
 #include "itexturetoolmodel.h"
 #include "icommandsystem.h"
 #include "TextureToolManipulationPivot.h"
@@ -23,6 +24,8 @@ class TextureToolSelectionSystem :
     sigc::signal<void, selection::IManipulator::Type> _sigActiveManipulatorChanged;
     sigc::signal<void, SelectionMode> _sigSelectionModeChanged;
 
+    sigc::signal<void> _sigSelectionChanged;
+
     TextureToolManipulationPivot _manipulationPivot;
 
     std::size_t _unselectListener;
@@ -43,6 +46,8 @@ class TextureToolSelectionSystem :
     std::size_t countSelected() override;
     std::size_t countSelectedComponentNodes() override;
 
+    sigc::signal<void>& signal_selectionChanged() override;
+
     void clearSelection() override;
     void clearComponentSelection() override;
 
@@ -66,6 +71,8 @@ class TextureToolSelectionSystem :
     void onManipulationFinished() override;
     void onManipulationCancelled() override;
 
+    void onNodeSelectionChanged(ISelectable& selectable) override;
+
 private:
     void handleUnselectRequest(selection::UnselectSelectionRequest& request);
 

test/TextureTool.cpp

@@ -71,6 +71,38 @@ std::vector<textool::INode::Ptr> getAllSelectedComponentNodes()
     return selectedNodes;
 }
 
+class SelectionChangedCatcher
+{
+private:
+    bool _signalFired;
+    sigc::connection _connection;
+
+public:
+    SelectionChangedCatcher() :
+        _signalFired(false)
+    {
+        _connection = GlobalTextureToolSelectionSystem().signal_selectionChanged().connect([this]
+        {
+            _signalFired = true;
+        });
+    }
+
+    bool signalHasFired() const
+    {
+        return _signalFired;
+    }
+
+    void reset()
+    {
+        _signalFired = false;
+    }
+
+    ~SelectionChangedCatcher()
+    {
+        _connection.disconnect();
+    }
+};
+
 // Checks that changing the regular scene selection will have an effect on the tex tool scene
 TEST_F(TextureToolTest, SceneGraphObservesSelection)
 {
@@ -489,9 +521,13 @@ TEST_F(TextureToolTest, TestSelectPatchSurfaceByPoint)
     render::TextureToolView view;
     view.constructFromTextureSpaceBounds(bounds, TEXTOOL_WIDTH, TEXTOOL_HEIGHT);
 
+    SelectionChangedCatcher signalObserver;
+
     // Test-select in the middle of the patch bounds
     performPointSelection(Vector2(bounds.origin.x(), bounds.origin.y()), view);
 
+    EXPECT_TRUE(signalObserver.signalHasFired()) << "No selection changed signal emitted";
+
     // Check if the node was selected
     auto selectedNodes = getAllSelectedTextoolNodes();
     EXPECT_EQ(selectedNodes.size(), 1) << "Only one patch should be selected";
@@ -519,24 +555,37 @@ TEST_F(TextureToolTest, TestSelectPatchVertexByPoint)
     auto firstVertex = patch->ctrlAt(2, 1).texcoord;
     auto secondVertex = patch->ctrlAt(2, 0).texcoord;
 
+    SelectionChangedCatcher signalObserver;
+
     // Selecting something in the middle of two vertices should not do anything
     performPointSelection((firstVertex + secondVertex) / 2, view);
     EXPECT_TRUE(getAllSelectedComponentNodes().empty()) << "Test-selecting a patch in between vertices should not have succeeded";
-
+    EXPECT_FALSE(signalObserver.signalHasFired()) << "Selection Changed Signal shouldn't have fired";
+    signalObserver.reset();
+
     performPointSelection(firstVertex, view);
 
     // Hitting a vertex will select the patch componentselectable
     EXPECT_EQ(getAllSelectedComponentNodes().size(), 1) << "Only one patch should be selected";
+    EXPECT_TRUE(signalObserver.signalHasFired()) << "Selection Changed Signal should have fired";
+    signalObserver.reset();
 
     // Hitting another vertex should not de-select the componentselectable
     performPointSelection(secondVertex, view);
     EXPECT_EQ(getAllSelectedComponentNodes().size(), 1) << "Only one patch should still be selected";
+    EXPECT_TRUE(signalObserver.signalHasFired()) << "Selection Changed Signal should have fired";
+    signalObserver.reset();
 
     // De-selecting the first and the second vertex should release the patch
     performPointSelection(secondVertex, view);
     EXPECT_EQ(getAllSelectedComponentNodes().size(), 1) << "Only one patch should still be selected";
+    EXPECT_TRUE(signalObserver.signalHasFired()) << "Selection Changed Signal should have fired";
+    signalObserver.reset();
+
     performPointSelection(firstVertex, view);
     EXPECT_TRUE(getAllSelectedComponentNodes().empty()) << "Selection should be empty now";
+    EXPECT_TRUE(signalObserver.signalHasFired()) << "Selection Changed Signal should have fired";
+    signalObserver.reset();
 }
 
 TEST_F(TextureToolTest, TestSelectFaceSurfaceByPoint)
@@ -562,9 +611,13 @@ TEST_F(TextureToolTest, TestSelectFaceSurfaceByPoint)
     render::TextureToolView view;
     view.constructFromTextureSpaceBounds(bounds, TEXTOOL_WIDTH, TEXTOOL_HEIGHT);
 
+    SelectionChangedCatcher signalObserver;
+
     // Point-select in the middle of the face
     performPointSelection(algorithm::getFaceCentroid(faceUp), view);
 
+    EXPECT_TRUE(signalObserver.signalHasFired()) << "Selection Changed Signal should have fired";
+
     // Check if the node was selected
     auto selectedNodes = getAllSelectedTextoolNodes();
     EXPECT_EQ(selectedNodes.size(), 1) << "Only one item should be selected";
@@ -595,27 +648,40 @@ TEST_F(TextureToolTest, TestSelectFaceVertexByPoint)
     // Switch to vertex selection mode
     GlobalTextureToolSelectionSystem().setMode(textool::SelectionMode::Vertex);
 
+    SelectionChangedCatcher signalObserver;
+
     // Get the texcoords of the first vertex
     auto firstVertex = faceUp->getWinding()[0].texcoord;
     auto secondVertex = faceUp->getWinding()[1].texcoord;
 
     // Selecting something in the middle of two vertices should not do anything
     performPointSelection((firstVertex + secondVertex) / 2, view);
     EXPECT_TRUE(getAllSelectedComponentNodes().empty()) << "Test-selecting a face in between vertices should not have succeeded";
+    EXPECT_FALSE(signalObserver.signalHasFired()) << "Selection Changed Signal shouldn't have fired";
+    signalObserver.reset();
 
     // Hitting a vertex will select the face
     performPointSelection(firstVertex, view);
     EXPECT_EQ(getAllSelectedComponentNodes().size(), 1) << "Only one face should be selected";
+    EXPECT_TRUE(signalObserver.signalHasFired()) << "Selection Changed Signal should have fired";
+    signalObserver.reset();
 
     // Hitting another vertex should not de-select the face
     performPointSelection(secondVertex, view);
     EXPECT_EQ(getAllSelectedComponentNodes().size(), 1) << "Only one face should still be selected";
+    EXPECT_TRUE(signalObserver.signalHasFired()) << "Selection Changed Signal should have fired";
+    signalObserver.reset();
 
     // De-selecting the first and the second vertex should release the face
     performPointSelection(secondVertex, view);
     EXPECT_EQ(getAllSelectedComponentNodes().size(), 1) << "Only one face should still be selected";
+    EXPECT_TRUE(signalObserver.signalHasFired()) << "Selection Changed Signal should have fired";
+    signalObserver.reset();
+
     performPointSelection(firstVertex, view);
     EXPECT_TRUE(getAllSelectedComponentNodes().empty()) << "Selection should be empty now";
+    EXPECT_TRUE(signalObserver.signalHasFired()) << "Selection Changed Signal should have fired";
+    signalObserver.reset();
 }
 
 TEST_F(TextureToolTest, TestSelectPatchByArea)
@@ -636,8 +702,12 @@ TEST_F(TextureToolTest, TestSelectPatchByArea)
     ConstructSelectionTest(view, selection::Rectangle::ConstructFromArea(Vector2(-0.95f, -0.95f), Vector2(0.95f*2, 0.95f*2)));
 
     SelectionVolume test(view);
+    SelectionChangedCatcher signalObserver;
+
     GlobalTextureToolSelectionSystem().selectArea(test, SelectionSystem::eToggle);
 
+    EXPECT_TRUE(signalObserver.signalHasFired()) << "Selection Changed Signal should have fired";
+
     // Check if the node was selected
     auto selectedNodes = getAllSelectedTextoolNodes();
     EXPECT_EQ(selectedNodes.size(), 1) << "Only one patch should be selected";
@@ -705,28 +775,37 @@ TEST_F(TextureToolTest, ClearSelectionUsingCommand)
     EXPECT_GT(GlobalTextureToolSelectionSystem().countSelectedComponentNodes(), 0) << "No components selected";
     EXPECT_GT(GlobalSelectionSystem().countSelected(), 0) << "Scene selection count should be > 0";
 
+    SelectionChangedCatcher signalObserver;
+
     // Hitting ESC once will deselect the components
     GlobalCommandSystem().executeCommand("UnSelectSelection");
 
     EXPECT_EQ(GlobalTextureToolSelectionSystem().countSelectedComponentNodes(), 0) << "Component selection should be gone";
     EXPECT_GT(GlobalTextureToolSelectionSystem().countSelected(), 0) << "Surface selection should not have been touched";
     EXPECT_GT(GlobalSelectionSystem().countSelected(), 0) << "Scene selection count should still be > 0";
     EXPECT_EQ(GlobalTextureToolSelectionSystem().getMode(), textool::SelectionMode::Vertex) << "We should still be in vertex mode";
+    EXPECT_TRUE(signalObserver.signalHasFired()) << "Selection Changed Signal should have fired";
+    signalObserver.reset();
 
     // Next deselection will exit vertex mode
     GlobalCommandSystem().executeCommand("UnSelectSelection");
     EXPECT_EQ(GlobalTextureToolSelectionSystem().getMode(), textool::SelectionMode::Surface) << "We should be in Surface mode now";
     EXPECT_GT(GlobalTextureToolSelectionSystem().countSelected(), 0) << "Surface selection should not have been touched";
     EXPECT_GT(GlobalSelectionSystem().countSelected(), 0) << "Scene selection count should still be > 0";
+    EXPECT_FALSE(signalObserver.signalHasFired()) << "Selection Changed Signal shouldn't have fired";
+    signalObserver.reset();
 
     // Next will de-select the regular selection
     GlobalCommandSystem().executeCommand("UnSelectSelection");
     EXPECT_EQ(GlobalTextureToolSelectionSystem().countSelected(), 0) << "Surface selection should be gone now";
     EXPECT_GT(GlobalSelectionSystem().countSelected(), 0) << "Scene selection count should still be > 0";
+    EXPECT_TRUE(signalObserver.signalHasFired()) << "Selection Changed Signal should have fired";
+    signalObserver.reset();
 
     // Now that the tex tool selection is gone, we should affect the scene selection
     GlobalCommandSystem().executeCommand("UnSelectSelection");
     EXPECT_EQ(GlobalSelectionSystem().countSelected(), 0) << "Scene selection should be gone now";
+    EXPECT_FALSE(signalObserver.signalHasFired()) << "Selection Changed Signal shouldn't have fired";
 }
 
 TEST_F(TextureToolTest, ClearSelection)
@@ -755,9 +834,13 @@ TEST_F(TextureToolTest, ClearSelection)
     // We should have a non-empty selection
     EXPECT_GT(GlobalTextureToolSelectionSystem().countSelected(), 0) << "No nodes selected";
 
+    SelectionChangedCatcher signalObserver;
+
     // Deselect
     GlobalTextureToolSelectionSystem().clearSelection();
 
+    EXPECT_TRUE(signalObserver.signalHasFired()) << "Selection Changed Signal should have fired";
+
     EXPECT_EQ(GlobalTextureToolSelectionSystem().countSelected(), 0) << "Surface selection should be gone now";
     EXPECT_EQ(GlobalSelectionSystem().countSelected(), 3) << "3 scene nodes must be selected";
 
@@ -789,9 +872,12 @@ TEST_F(TextureToolTest, ClearComponentSelection)
 
     EXPECT_EQ(GlobalTextureToolSelectionSystem().countSelectedComponentNodes(), 1) << "We should have 1 selected component node";
 
+    SelectionChangedCatcher signalObserver;
+
     // Deselect all components
     GlobalTextureToolSelectionSystem().clearComponentSelection();
 
+    EXPECT_TRUE(signalObserver.signalHasFired()) << "Selection Changed Signal should have fired";
     EXPECT_EQ(GlobalTextureToolSelectionSystem().countSelectedComponentNodes(), 0) << "Component selection should be gone now";
     EXPECT_EQ(GlobalTextureToolSelectionSystem().getMode(), textool::SelectionMode::Vertex) << "Should still be in vertex mode";
 }