Skip to content

Commit

Permalink
#5807: Add Face destruction signal, since there's no other easy way t…
Browse files Browse the repository at this point in the history
…o get notified about Faces being removed from a selected brush.
  • Loading branch information
codereader committed Nov 12, 2021
1 parent f425325 commit f705f8d
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 6 deletions.
3 changes: 3 additions & 0 deletions include/ibrush.h
Expand Up @@ -206,6 +206,9 @@ class IFace

// Transforms this face plane with the given transformation matrix
virtual void transform(const Matrix4& transformation) = 0;

// Emitted from this IFace's destructor, as last sign of life
virtual sigc::signal<void>& signal_faceDestroyed() = 0;
};

// Plane classification info used by splitting and CSG algorithms
Expand Down
7 changes: 7 additions & 0 deletions radiantcore/brush/Face.cpp
Expand Up @@ -111,6 +111,13 @@ Face::Face(Brush& owner, const Face& other) :
Face::~Face()
{
_surfaceShaderRealised.disconnect();
_sigDestroyed.emit();
_sigDestroyed.clear();
}

sigc::signal<void>& Face::signal_faceDestroyed()
{
return _sigDestroyed;
}

void Face::setupSurfaceShader()
Expand Down
4 changes: 4 additions & 0 deletions radiantcore/brush/Face.h
Expand Up @@ -60,6 +60,8 @@ class Face :
// Cached visibility flag, queried during front end rendering
bool _faceIsVisible;

sigc::signal<void> _sigDestroyed;

public:

// Constructors
Expand All @@ -81,6 +83,8 @@ class Face :

Brush& getBrushInternal();

sigc::signal<void>& signal_faceDestroyed() override;

void planeChanged();

// greebo: Emits the updated normals to the Winding class.
Expand Down
30 changes: 26 additions & 4 deletions radiantcore/selection/textool/TextureToolSceneGraph.cpp
Expand Up @@ -6,6 +6,7 @@
#include "module/StaticModule.h"
#include "selectionlib.h"

#include "selection/algorithm/Primitives.h"
#include "FaceNode.h"
#include "PatchNode.h"

Expand Down Expand Up @@ -47,6 +48,7 @@ void TextureToolSceneGraph::shutdownModule()
{
_selectionNeedsRescan = false;
_activeMaterialNeedsRescan = false;
clearFaceObservers();
_nodes.clear();
_sceneSelectionChanged.disconnect();
GlobalRadiantCore().getMessageBus().removeListener(_textureChangedHandler);
Expand Down Expand Up @@ -93,17 +95,17 @@ void TextureToolSceneGraph::ensureSceneIsAnalysed()
if (!_selectionNeedsRescan) return;

_selectionNeedsRescan = false;
clearFaceObservers();
_nodes.clear();

// No unique material, leave everything empty
if (_activeMaterial.empty()) return;

if (GlobalSelectionSystem().countSelectedComponents() > 0)
{
// Check each selected face
GlobalSelectionSystem().foreachFace([&](IFace& face)
selection::algorithm::forEachSelectedFaceComponent([&](IFace& face)
{
_nodes.emplace_back(std::make_shared<FaceNode>(face));
createFaceNode(face);
});
}

Expand All @@ -116,7 +118,7 @@ void TextureToolSceneGraph::ensureSceneIsAnalysed()

for (auto i = 0; i < brush->getNumFaces(); ++i)
{
_nodes.emplace_back(std::make_shared<FaceNode>(brush->getFace(i)));
createFaceNode(brush->getFace(i));
}
}
else if (Node_isPatch(node))
Expand All @@ -126,6 +128,26 @@ void TextureToolSceneGraph::ensureSceneIsAnalysed()
});
}

void TextureToolSceneGraph::clearFaceObservers()
{
for (auto& conn : _faceObservers)
{
conn.disconnect();
}

_faceObservers.clear();
}

void TextureToolSceneGraph::createFaceNode(IFace& face)
{
_nodes.emplace_back(std::make_shared<FaceNode>(face));

// When faces are destroyed, the selection is out of date, queue a rescan
_faceObservers.emplace_back(face.signal_faceDestroyed().connect(
[this]() { _selectionNeedsRescan = true; }
));
}

void TextureToolSceneGraph::onSceneSelectionChanged(const ISelectable& selectable)
{
// Mark our own selection as dirty
Expand Down
3 changes: 3 additions & 0 deletions radiantcore/selection/textool/TextureToolSceneGraph.h
Expand Up @@ -19,6 +19,7 @@ class TextureToolSceneGraph :
bool _activeMaterialNeedsRescan;

std::list<INode::Ptr> _nodes;
std::vector<sigc::connection> _faceObservers;

// The single active material. Is empty if the scene graph has no items
std::string _activeMaterial;
Expand All @@ -38,6 +39,8 @@ class TextureToolSceneGraph :
private:
void onSceneSelectionChanged(const ISelectable& selectable);
void onTextureChanged(radiant::TextureChangedMessage& msg);
void createFaceNode(IFace& face);
void clearFaceObservers();

void ensureSceneIsAnalysed();
};
Expand Down
19 changes: 17 additions & 2 deletions test/TextureTool.cpp
Expand Up @@ -209,14 +209,14 @@ TEST_F(TextureToolTest, SceneGraphRecognisesSingleFaces)
GlobalSelectionSystem().selectPoint(testFaceUp, selection::SelectionSystem::eToggle, true);

EXPECT_EQ(GlobalSelectionSystem().countSelectedComponents(), 1) << "1 Face component should be selected";
EXPECT_EQ(getTextureToolNodeCount(), 1) << "There should be some exactly 1 tex tool node now";
EXPECT_EQ(getTextureToolNodeCount(), 1) << "There should be exactly 1 tex tool node in the scene";

Node_setSelected(brush2, true);

EXPECT_EQ(GlobalSelectionSystem().countSelectedComponents(), 1) << "1 Face component should be selected";
EXPECT_EQ(GlobalSelectionSystem().countSelected(), 1) << "1 Brush should be selected";

EXPECT_EQ(getTextureToolNodeCount(), 7) << "There should be 7 tex tool nodes now, 1 single face + 6 brush faces";
EXPECT_EQ(getTextureToolNodeCount(), 7) << "There should be 7 tex tool nodes in the scene, 1 single face + 6 brush faces";
}

TEST_F(TextureToolTest, SceneGraphRecognisesPatches)
Expand All @@ -232,6 +232,21 @@ TEST_F(TextureToolTest, SceneGraphRecognisesPatches)
EXPECT_GT(getTextureToolNodeCount(), 0) << "There should be some tex tool nodes now";
}

TEST_F(TextureToolTest, SceneGraphNotifiedAboutFaceDestruction)
{
auto worldspawn = GlobalMapModule().findOrInsertWorldspawn();
auto brush1 = algorithm::createCubicBrush(worldspawn, Vector3(0, 0, 0), "textures/numbers/1");
Node_setSelected(brush1, true);

EXPECT_EQ(GlobalSelectionSystem().countSelected(), 1) << "1 Brush should be selected";
EXPECT_EQ(getTextureToolNodeCount(), 6) << "There should be exactly 6 tex tool nodes in the scene";

Node_getIBrush(brush1)->clear();

EXPECT_EQ(GlobalSelectionSystem().countSelected(), 1) << "Technically, 1 Brush should still be selected";
EXPECT_EQ(getTextureToolNodeCount(), 0) << "All faces have been cleared, tex tool scene should be empty now";
}

// Selecting an inhomogenously textured brush will result in an empty scene graph
TEST_F(TextureToolTest, SceneGraphSkipsInhomogeneousBrushes)
{
Expand Down

0 comments on commit f705f8d

Please sign in to comment.