3214: update face tags when the brush changes (#3218)

Fixes #3214 Note that Brush::doSetNewGeometry clones the BrushFaces (which doesn't clone the Taggable info per face).
TrenchBroom · May 19, 2020 · cf27707 · cf27707
1 parent 08fb5e7
commit cf27707
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 3 deletions.
diff --git a/common/src/Model/Brush.cpp b/common/src/Model/Brush.cpp
@@ -1549,6 +1549,13 @@ namespace TrenchBroom {
             Taggable::clearTags();
         }
 
+        void Brush::updateTags(TagManager& tagManager) {
+            for (auto* face : m_faces) {
+                face->updateTags(tagManager);
+            }
+            Taggable::updateTags(tagManager);
+        }
+
         bool Brush::allFacesHaveAnyTagInMask(TagType::Type tagMask) const {
             // Possible optimization: Store the shared face tag mask in the brush and updated it when a face changes.
 

diff --git a/common/src/Model/Brush.h b/common/src/Model/Brush.h
@@ -313,6 +313,7 @@ namespace TrenchBroom {
         public:
             void initializeTags(TagManager& tagManager) override;
             void clearTags() override;
+            void updateTags(TagManager& tagManager) override;
 
             /**
              * Indicates whether all of the faces of this brush have any of the given tags.

diff --git a/common/test/src/View/TagManagementTest.cpp b/common/test/src/View/TagManagementTest.cpp
@@ -471,20 +471,30 @@ namespace TrenchBroom {
             ASSERT_TRUE(brush->hasTag(tag));
         }
 
-        TEST_CASE_METHOD(TagManagementTest, "TagManagementTest.tagInitializeBrushFaceTags") {
+        TEST_CASE_METHOD(TagManagementTest, "TagManagementTest.tagInitializeBrushFaceTags", "[TagManagementTest]") {
             auto* brushWithTags = createBrush("some_texture");
             document->addNode(brushWithTags, document->currentParent());
+            document->select(brushWithTags);
+
+            SECTION("No modification to brush") {
+            }
+            SECTION("Vertex manipulation") {
+                const auto verticesToMove = std::map<vm::vec3, std::vector<Model::Brush*>>{ { vm::vec3::fill(16.0), { brushWithTags } } };
+                const auto result = document->moveVertices(verticesToMove, vm::vec3::fill(1.0));
+                REQUIRE(result.success);
+                REQUIRE(result.hasRemainingVertices);
+            }
 
             const auto& tag = document->smartTag("texture");
             for (const auto* face : brushWithTags->faces()) {
-                ASSERT_TRUE(face->hasTag(tag));
+                CHECK(face->hasTag(tag));
             }
 
             auto* brushWithoutTags = createBrush("asdf");
             document->addNode(brushWithoutTags, document->currentParent());
 
             for (const auto* face : brushWithoutTags->faces()) {
-                ASSERT_FALSE(face->hasTag(tag));
+                CHECK(!face->hasTag(tag));
             }
         }