diff --git a/libs/scene/SceneGraphComparer.cpp b/libs/scene/SceneGraphComparer.cpp index 3022f06c46..d7475ad10d 100644 --- a/libs/scene/SceneGraphComparer.cpp +++ b/libs/scene/SceneGraphComparer.cpp @@ -11,6 +11,16 @@ namespace scene { +namespace +{ + inline std::string getEntityName(const scene::INodePtr& node) + { + auto entity = Node_getEntity(node); + + return entity->isWorldspawn() ? "worldspawn" : entity->getKeyValue("name"); + } +} + void SceneGraphComparer::compare() { auto sourceEntities = collectEntityFingerprints(_source); @@ -37,8 +47,7 @@ void SceneGraphComparer::compare() } else { - auto entityName = Node_getEntity(sourceEntity.second)->getKeyValue("name"); - + auto entityName = getEntityName(sourceEntity.second); sourceMismatches.emplace(entityName, EntityMismatch{ sourceEntity.first, sourceEntity.second, entityName }); } } @@ -51,8 +60,7 @@ void SceneGraphComparer::compare() // Matching nodes have already been checked in the above loop if (sourceEntities.count(baseEntity.first) == 0) { - auto entityName = Node_getEntity(baseEntity.second)->getKeyValue("name"); - + auto entityName = getEntityName(baseEntity.second); baseMismatches.emplace(entityName, EntityMismatch{ baseEntity.first, baseEntity.second, entityName }); } } @@ -102,6 +110,9 @@ void SceneGraphComparer::processDifferingEntities(const EntityMismatchByName& so // Analyse the key values entityDiff.differingKeyValues = compareKeyValues(sourceNode, baseNode); + + // Analyse the child nodes + entityDiff.differingChildren = compareChildNodes(sourceNode, baseNode); } for (const auto& mismatch : missingInSource) @@ -210,18 +221,61 @@ std::list SceneGraphComparer::compareKeyVa return result; } -SceneGraphComparer::Fingerprints SceneGraphComparer::collectEntityFingerprints(const scene::INodePtr& root) +std::list SceneGraphComparer::compareChildNodes( + const scene::INodePtr& sourceNode, const scene::INodePtr& baseNode) { - Fingerprints result; + std::list result; + + auto sourceChildren = collectPrimitiveFingerprints(sourceNode); + auto baseChildren = collectPrimitiveFingerprints(baseNode); - root->foreachNode([&](const scene::INodePtr& node) + std::vector missingInSource; + std::vector missingInBase; + + auto compareFingerprint = [](const Fingerprints::value_type& left, const Fingerprints::value_type& right) + { + return left.first < right.first; + }; + + std::set_difference(sourceChildren.begin(), sourceChildren.end(), + baseChildren.begin(), baseChildren.end(), std::back_inserter(missingInBase), compareFingerprint); + std::set_difference(baseChildren.begin(), baseChildren.end(), + sourceChildren.begin(), sourceChildren.end(), std::back_inserter(missingInSource), compareFingerprint); + + for (const auto& pair : missingInBase) { - if (node->getNodeType() != scene::INode::Type::Entity) + result.emplace_back(ComparisonResult::PrimitiveDifference { - return true; - } + pair.first, + pair.second, + ComparisonResult::PrimitiveDifference::Type::PrimitiveAdded + }); + } + + for (const auto& pair : missingInSource) + { + result.emplace_back(ComparisonResult::PrimitiveDifference + { + pair.first, + pair.second, + ComparisonResult::PrimitiveDifference::Type::PrimitiveRemoved + }); + } + + return result; +} + +SceneGraphComparer::Fingerprints SceneGraphComparer::collectNodeFingerprints(const scene::INodePtr& parent, + const std::function& nodePredicate) +{ + Fingerprints result; + + parent->foreachNode([&](const scene::INodePtr& node) + { + if (!nodePredicate(node)) return true; // predicate says "skip" auto comparable = std::dynamic_pointer_cast(node); + assert(comparable); if (!comparable) return true; // skip @@ -230,7 +284,7 @@ SceneGraphComparer::Fingerprints SceneGraphComparer::collectEntityFingerprints(c if (!insertResult.second) { - rWarning() << "More than one entity with the same fingerprint found in the map " << node->name() << std::endl; + rWarning() << "More than one node with the same fingerprint found in the parent node with name " << parent->name() << std::endl; } return true; @@ -239,4 +293,20 @@ SceneGraphComparer::Fingerprints SceneGraphComparer::collectEntityFingerprints(c return result; } +SceneGraphComparer::Fingerprints SceneGraphComparer::collectPrimitiveFingerprints(const scene::INodePtr& parent) +{ + return collectNodeFingerprints(parent, [](const scene::INodePtr& node) + { + return node->getNodeType() == scene::INode::Type::Brush || node->getNodeType() == scene::INode::Type::Patch; + }); +} + +SceneGraphComparer::Fingerprints SceneGraphComparer::collectEntityFingerprints(const scene::INodePtr& root) +{ + return collectNodeFingerprints(root, [](const scene::INodePtr& node) + { + return node->getNodeType() == scene::INode::Type::Entity; + }); +} + } diff --git a/libs/scene/SceneGraphComparer.h b/libs/scene/SceneGraphComparer.h index 8ef2cbe8a5..aa289aee03 100644 --- a/libs/scene/SceneGraphComparer.h +++ b/libs/scene/SceneGraphComparer.h @@ -10,10 +10,18 @@ namespace scene { -struct ComparisonResult +class ComparisonResult { +public: using Ptr = std::shared_ptr; +private: + // This result instance will hold references to the root nodes of the compared graph + // to ensure the graph stays alive as long as the result + scene::IMapRootNodePtr _sourceRoot; + scene::IMapRootNodePtr _baseRoot; + +public: // Represents a matching node pair struct Match { @@ -29,14 +37,28 @@ struct ComparisonResult enum class Type { - KeyValueAdded, // key is present on the source entity, but not on the target - KeyValueRemoved, // key is present on the target entity, but not on the source + KeyValueAdded, // key is present on the source entity, but not on the base + KeyValueRemoved, // key is present on the base entity, but not on the source KeyValueChanged, // key present on both, but value is different }; Type type; }; + struct PrimitiveDifference + { + std::string fingerprint; + scene::INodePtr node; + + enum class Type + { + PrimitiveAdded, // child is present on the source entity, but not on the base + PrimitiveRemoved, // child is present on the base entity, but not on the source + }; + + Type type; + }; + struct EntityDifference { std::string fingerprint; @@ -53,8 +75,16 @@ struct ComparisonResult Type type; std::list differingKeyValues; + + std::list differingChildren; }; +public: + ComparisonResult(const scene::IMapRootNodePtr& sourceRoot, const scene::IMapRootNodePtr& baseRoot) : + _sourceRoot(sourceRoot), + _baseRoot(baseRoot) + {} + // The collection of entities with the same fingerprint value std::list equivalentEntities; @@ -86,7 +116,7 @@ class SceneGraphComparer SceneGraphComparer(const scene::IMapRootNodePtr& source, const scene::IMapRootNodePtr& base) : _source(source), _base(base), - _result(new ComparisonResult) + _result(new ComparisonResult(_source, _base)) {} void compare(); @@ -97,12 +127,19 @@ class SceneGraphComparer } private: - void processDifferingEntities(const EntityMismatchByName& sourceMismatches, const EntityMismatchByName& targetMismatches); + void processDifferingEntities(const EntityMismatchByName& sourceMismatches, const EntityMismatchByName& baseMismatches); Fingerprints collectEntityFingerprints(const scene::INodePtr& root); + Fingerprints collectPrimitiveFingerprints(const scene::INodePtr& parent); + + Fingerprints collectNodeFingerprints(const scene::INodePtr& parent, + const std::function& nodePredicate); std::list compareKeyValues( - const scene::INodePtr& sourceNode, const scene::INodePtr& targetNode); + const scene::INodePtr& sourceNode, const scene::INodePtr& baseNode); + + std::list compareChildNodes( + const scene::INodePtr& sourceNode, const scene::INodePtr& baseNode); }; } diff --git a/test/MapMerging.cpp b/test/MapMerging.cpp index cf3a7d863a..170eb0fd88 100644 --- a/test/MapMerging.cpp +++ b/test/MapMerging.cpp @@ -293,6 +293,7 @@ TEST_F(MapMergeTest, DetectAddedEntities) // light_3 start has been added to the changed map EXPECT_TRUE(resultHasEntityDifference(result, "light_3", scene::ComparisonResult::EntityDifference::Type::EntityMissingInBase)); + EXPECT_TRUE(resultHasEntityDifference(result, "func_static_2", scene::ComparisonResult::EntityDifference::Type::EntityMissingInBase)); } TEST_F(MapMergeTest, DetectAddedKeyValues) @@ -333,4 +334,19 @@ TEST_F(MapMergeTest, DetectChangedKeyValues) EXPECT_TRUE(hasKeyValueDifference(diff, "origin", "280 160 0", scene::ComparisonResult::KeyValueDifference::Type::KeyValueChanged)); } +TEST_F(MapMergeTest, DetectAddedChildPrimitives) +{ + auto result = performComparison("maps/fingerprinting.mapx", _context.getTestProjectPath() + "maps/fingerprinting_2.mapx"); + + // func_static_1 has changed primitived + auto diff = getEntityDifference(result, "func_static_30"); + + EXPECT_EQ(diff.type, scene::ComparisonResult::EntityDifference::Type::EntityPresentButDifferent); + EXPECT_EQ(diff.differingChildren.size(), 1); + EXPECT_EQ(diff.differingChildren.front().type, scene::ComparisonResult::PrimitiveDifference::Type::PrimitiveAdded); + EXPECT_EQ(diff.differingChildren.front().node->getNodeType(), scene::INode::Type::Brush); + + +} + } diff --git a/test/resources/tdm/maps/fingerprinting.mapx b/test/resources/tdm/maps/fingerprinting.mapx index 40f279ae14..d45effdc28 100644 --- a/test/resources/tdm/maps/fingerprinting.mapx +++ b/test/resources/tdm/maps/fingerprinting.mapx @@ -8,53 +8,14 @@ - - - + + + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - @@ -93,7 +54,7 @@ - + @@ -132,7 +93,7 @@ - + @@ -171,7 +132,7 @@ - + @@ -190,7 +151,7 @@ - + @@ -209,6 +170,45 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -335,10 +335,10 @@ + - @@ -351,8 +351,218 @@ - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/resources/tdm/maps/fingerprinting_2.mapx b/test/resources/tdm/maps/fingerprinting_2.mapx index 1a6ae97acb..ee6ba40b40 100644 --- a/test/resources/tdm/maps/fingerprinting_2.mapx +++ b/test/resources/tdm/maps/fingerprinting_2.mapx @@ -8,10 +8,10 @@ - - - - + + + + @@ -55,45 +55,6 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - @@ -132,7 +93,7 @@ - + @@ -171,7 +132,7 @@ - + @@ -190,7 +151,7 @@ - + @@ -209,6 +170,45 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -300,6 +300,51 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -320,10 +365,10 @@ + - @@ -361,4 +406,338 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +