Add new signal to LayerSystem to get notified about membership changes

Plus some refactorings

include/ilayer.h

@@ -182,14 +182,23 @@ class ILayerSystem :
 
 	/**
 	 * A signal for client code to get notified about layer creation,
-	 * addition, removal.
+	 * renamings and removal.
 	 */
 	virtual sigc::signal<void> signal_layersChanged() = 0;
 
 	/**
-	 *
+	 * Fired whenever visibility of a layer has been changed.
 	 */
 	virtual sigc::signal<void> signal_layerVisibilityChanged() = 0;
+
+	/**
+	 * Public signal to get notified about layer membership changes,
+	 * e.g. when a node has been added to a layer, or moved to a new one.
+	 * Since scene::INodes can be added or removed from layers directly,
+	 * without the LayerSystem knowing about this, it is sometimes the
+	 * responsibility of that algorithm code to emit this signal itself.
+	 */
+	virtual sigc::signal<void> signal_nodeMembershipChanged() = 0;
 };
 
 } // namespace scene

radiant/layers/LayerSystem.cpp

@@ -65,6 +65,9 @@ int LayerSystem::createLayer(const std::string& name, int layerID)
 	// Set the newly created layer to "visible"
 	_layerVisibility[result.first->first] = true;
 
+	// Layers have changed
+	onLayersChanged();
+
 	// Return the ID of the inserted layer
 	return result.first->first;
 }
@@ -115,9 +118,12 @@ void LayerSystem::deleteLayer(const std::string& name)
 		_activeLayer = DEFAULT_LAYER;
 	}
 
-	// Fire the visibility changed event to
-	// update the scenegraph and redraw the views
-	onLayerVisibilityChanged();
+	// Layers have changed
+	onLayersChanged();
+
+	// Nodes might have switched to default, fire the visibility 
+	// changed event, update the scenegraph and redraw the views
+	onNodeMembershipChanged();
 }
 
 void LayerSystem::foreachLayer(const LayerVisitFunc& visitor)
@@ -159,6 +165,9 @@ bool LayerSystem::renameLayer(int layerID, const std::string& newLayerName)
 	// Rename that layer
 	i->second = newLayerName;
 
+	// Fire the update signal
+	onLayersChanged();
+
 	return true;
 }
 
@@ -265,18 +274,31 @@ void LayerSystem::setLayerVisibility(const std::string& layerName, bool visible)
 	setLayerVisibility(layerID, visible);
 }
 
-void LayerSystem::updateSceneGraphVisibility() {
+void LayerSystem::updateSceneGraphVisibility()
+{
 	UpdateNodeVisibilityWalker walker;
 	GlobalSceneGraph().root()->traverseChildren(walker);
+
+	// Redraw
+	SceneChangeNotify();
 }
 
-void LayerSystem::onLayerVisibilityChanged()
+void LayerSystem::onLayersChanged()
 {
-	// Update all nodes
+	_layersChangedSignal.emit();
+}
+
+void LayerSystem::onNodeMembershipChanged()
+{
+	_nodeMembershipChangedSignal.emit();
+
 	updateSceneGraphVisibility();
+}
 
-	// Redraw
-	SceneChangeNotify();
+void LayerSystem::onLayerVisibilityChanged()
+{
+	// Update all nodes and views
+	updateSceneGraphVisibility();
 
 	// Update the LayerControlDialog
 	_layerVisibilityChangedSignal.emit();
@@ -292,7 +314,7 @@ void LayerSystem::addSelectionToLayer(int layerID) {
 	AddToLayerWalker walker(layerID);
 	GlobalSelectionSystem().foreachSelected(walker);
 
-	updateSceneGraphVisibility();
+	onNodeMembershipChanged();
 }
 
 void LayerSystem::addSelectionToLayer(const std::string& layerName) {
@@ -333,7 +355,7 @@ void LayerSystem::moveSelectionToLayer(int layerID) {
 	MoveToLayerWalker walker(layerID);
 	GlobalSelectionSystem().foreachSelected(walker);
 
-	updateSceneGraphVisibility();
+	onNodeMembershipChanged();
 }
 
 void LayerSystem::removeSelectionFromLayer(const std::string& layerName) {
@@ -360,21 +382,24 @@ void LayerSystem::removeSelectionFromLayer(int layerID) {
 	RemoveFromLayerWalker walker(layerID);
 	GlobalSelectionSystem().foreachSelected(walker);
 
-	updateSceneGraphVisibility();
+	onNodeMembershipChanged();
 }
 
-bool LayerSystem::updateNodeVisibility(const scene::INodePtr& node) {
+bool LayerSystem::updateNodeVisibility(const scene::INodePtr& node)
+{
 	// Get the list of layers the node is associated with
-	// greebo: TODO: Check if returning the LayerList by value is taxing.
+	// greebo: FIXME: Check if returning the LayerList by value is taxing.
 	LayerList layers = node->getLayers();
 
 	// We start with the assumption that a node is hidden
 	node->enable(Node::eLayered);
 
 	// Cycle through the Node's layers, and show the node as soon as
 	// a visible layer is found.
-	for (LayerList::const_iterator i = layers.begin(); i != layers.end(); i++) {
-		if (_layerVisibility[*i]) {
+	for (int layerId : layers)
+	{
+		if (_layerVisibility[layerId])
+		{
 			// The layer is visible, set the visibility to true and quit
 			node->disable(Node::eLayered);
 			return true;
@@ -385,7 +410,8 @@ bool LayerSystem::updateNodeVisibility(const scene::INodePtr& node) {
 	return false;
 }
 
-void LayerSystem::setSelected(int layerID, bool selected) {
+void LayerSystem::setSelected(int layerID, bool selected)
+{
 	SetLayerSelectedWalker walker(layerID, selected);
 
     if (GlobalSceneGraph().root())
@@ -404,7 +430,13 @@ sigc::signal<void> LayerSystem::signal_layerVisibilityChanged()
 	return _layerVisibilityChangedSignal;
 }
 
-int LayerSystem::getLayerID(const std::string& name) const {
+sigc::signal<void> LayerSystem::signal_nodeMembershipChanged()
+{
+	return _nodeMembershipChangedSignal;
+}
+
+int LayerSystem::getLayerID(const std::string& name) const
+{
 	for (LayerMap::const_iterator i = _layers.begin(); i != _layers.end(); i++) {
 		if (i->second == name) {
 			// Name found, return the ID
@@ -415,7 +447,8 @@ int LayerSystem::getLayerID(const std::string& name) const {
 	return -1;
 }
 
-std::string LayerSystem::getLayerName(int layerID) const {
+std::string LayerSystem::getLayerName(int layerID) const 
+{
 	LayerMap::const_iterator found = _layers.find(layerID);
 
 	if (found != _layers.end()) {
@@ -431,7 +464,8 @@ bool LayerSystem::layerExists(int layerID) const
 	return _layers.find(layerID) != _layers.end();
 }
 
-int LayerSystem::getHighestLayerID() const {
+int LayerSystem::getHighestLayerID() const
+{
 	if (_layers.size() == 0) {
 		// Empty layer map, just return DEFAULT_LAYER
 		return DEFAULT_LAYER;
@@ -441,7 +475,8 @@ int LayerSystem::getHighestLayerID() const {
 	return _layers.rbegin()->first;
 }
 
-int LayerSystem::getLowestUnusedLayerID() {
+int LayerSystem::getLowestUnusedLayerID()
+{
 	for (int i = 0; i < INT_MAX; i++) {
 		if (_layers.find(i) == _layers.end()) {
 			// Found a free ID
@@ -543,7 +578,6 @@ void LayerSystem::createLayerCmd(const cmd::ArgumentList& args)
 		if (layerID != -1)
 		{
 			// Success, break the loop
-			_layersChangedSignal.emit();
 			break;
 		}
 		else {

radiant/layers/LayerSystem.h

@@ -38,6 +38,7 @@ class LayerSystem :
 
 	sigc::signal<void> _layersChangedSignal;
 	sigc::signal<void> _layerVisibilityChangedSignal;
+	sigc::signal<void> _nodeMembershipChangedSignal;
 
 public:
 	LayerSystem();
@@ -126,6 +127,7 @@ class LayerSystem :
 
 	sigc::signal<void> signal_layersChanged() override;
 	sigc::signal<void> signal_layerVisibilityChanged() override;
+	sigc::signal<void> signal_nodeMembershipChanged() override;
 
 	// RegisterableModule implementation
 	const std::string& getName() const;
@@ -138,9 +140,15 @@ class LayerSystem :
 private:
 	void onMapEvent(IMap::MapEvent ev);
 
+	// Internal event emitter
+	void onLayersChanged();
+
 	// Internal event, updates the scenegraph
 	void onLayerVisibilityChanged();
 
+	// Internal event emitter
+	void onNodeMembershipChanged();
+
 	// Updates the visibility state of the entire scenegraph
 	void updateSceneGraphVisibility();
 

radiant/ui/layers/LayerControl.cpp

@@ -168,7 +168,6 @@ void LayerControl::onDelete(wxCommandEvent& ev)
 		scene::getLayerSystem().deleteLayer(
 			scene::getLayerSystem().getLayerName(_layerID)
 		);
-		LayerControlDialog::Instance().refresh();
 	}
 }
 
@@ -198,9 +197,8 @@ void LayerControl::onRename(wxCommandEvent& ev)
 
 		if (success)
 		{
-			// Reload the widgets, we're done here
-			update();
-			break;
+			// Stop here, the control might already have been destroyed
+			return;
 		}
 		else
 		{

radiant/ui/layers/LayerControlDialog.cpp

@@ -110,7 +110,7 @@ void LayerControlDialog::refresh()
     {
         // Create a new layercontrol for each visited layer
         // Store the object in a sorted container
-        sortedControls[layerName] = LayerControlPtr(new LayerControl(_dialogPanel, layerID));
+        sortedControls[layerName] = std::make_shared<LayerControl>(_dialogPanel, layerID);
     });
 
     // Assign all controls to the target vector, alphabetically sorted
@@ -121,19 +121,18 @@ void LayerControlDialog::refresh()
 
 	_controlContainer->SetRows(static_cast<int>(_layerControls.size()));
 
-	for (LayerControls::iterator i = _layerControls.begin();
-		 i != _layerControls.end(); ++i)
+	for (const LayerControlPtr& control : _layerControls)
 	{
-		_controlContainer->Add((*i)->getToggle(), 0);
-		_controlContainer->Add((*i)->getStatusWidget(), 0, wxEXPAND | wxTOP | wxBOTTOM, 1);
-		_controlContainer->Add((*i)->getLabelButton(), 0, wxEXPAND);
-		_controlContainer->Add((*i)->getButtons(), 0, wxEXPAND);
+		_controlContainer->Add(control->getToggle(), 0);
+		_controlContainer->Add(control->getStatusWidget(), 0, wxEXPAND | wxTOP | wxBOTTOM, 1);
+		_controlContainer->Add(control->getLabelButton(), 0, wxEXPAND);
+		_controlContainer->Add(control->getButtons(), 0, wxEXPAND);
 
-        if (i == _layerControls.begin())
+        if (control == _layerControls.front())
         {
             // Prevent setting the focus on the buttons at the bottom which lets the scrollbar 
             // of the window jump around (#4089), set the focus on the first button.
-            (*i)->getLabelButton()->SetFocus();
+			control->getLabelButton()->SetFocus();
         }
 	}
 
@@ -272,12 +271,19 @@ void LayerControlDialog::_preShow()
 	_layerVisibilityChangedSignal = GlobalLayerSystem().signal_layerVisibilityChanged().connect(
 		sigc::mem_fun(this, &LayerControlDialog::update));
 
+	// Node membership triggers a selection rescan
+	_nodeLayerMembershipChangedSignal = GlobalLayerSystem().signal_nodeMembershipChanged().connect([this]()
+	{
+		_rescanSelectionOnIdle = true;
+	});
+
 	// Re-populate the dialog
 	refresh();
 }
 
 void LayerControlDialog::_postHide()
 {
+	_nodeLayerMembershipChangedSignal.disconnect();
 	_layersChangedSignal.disconnect();
 	_layerVisibilityChangedSignal.disconnect();
 	_selectionChangedSignal.disconnect();

radiant/ui/layers/LayerControlDialog.h

@@ -38,6 +38,7 @@ class LayerControlDialog :
 	sigc::connection _selectionChangedSignal;
 	sigc::connection _layersChangedSignal;
 	sigc::connection _layerVisibilityChangedSignal;
+	sigc::connection _nodeLayerMembershipChangedSignal;
 
 public:
 	LayerControlDialog();