#5623: Refactor GraphComparer into a static utility class, since ther…

…e's no pressing reason to maintain an instance.

libs/scene/merge/GraphComparer.cpp

@@ -24,10 +24,12 @@ namespace
     }
 }
 
-void GraphComparer::compare()
+ComparisonResult::Ptr GraphComparer::Compare(const scene::IMapRootNodePtr& source, const scene::IMapRootNodePtr& base)
 {
-    auto sourceEntities = collectEntityFingerprints(_source);
-    auto baseEntities = collectEntityFingerprints(_base);
+    auto result = std::make_shared<ComparisonResult>(source, base);
+
+    auto sourceEntities = collectEntityFingerprints(source);
+    auto baseEntities = collectEntityFingerprints(base);
 
     // Filter out all the matching nodes and store them in the result
     if (sourceEntities.empty())
@@ -46,7 +48,7 @@ void GraphComparer::compare()
         if (matchingBaseNode != baseEntities.end())
         {
             // Found an equivalent node
-            _result->equivalentEntities.emplace_back(ComparisonResult::Match{ sourceEntity.first, sourceEntity.second, matchingBaseNode->second });
+            result->equivalentEntities.emplace_back(ComparisonResult::Match{ sourceEntity.first, sourceEntity.second, matchingBaseNode->second });
         }
         else
         {
@@ -68,14 +70,13 @@ void GraphComparer::compare()
         }
     }
 
-    rMessage() << "Mismatching Source Entities: " << sourceMismatches.size() << std::endl;
-    rMessage() << "Mismatching Base Entities: " << baseMismatches.size() << std::endl;
-
     // Enter the second stage and try to match entities and detailing diffs
-    processDifferingEntities(sourceMismatches, baseMismatches);
+    processDifferingEntities(*result, sourceMismatches, baseMismatches);
+
+    return result;
 }
 
-void GraphComparer::processDifferingEntities(const EntityMismatchByName& sourceMismatches, const EntityMismatchByName& baseMismatches)
+void GraphComparer::processDifferingEntities(ComparisonResult& result, const EntityMismatchByName& sourceMismatches, const EntityMismatchByName& baseMismatches)
 {
     // Find all entities that are missing in either source or base (by name)
     std::list<EntityMismatchByName::value_type> missingInSource;
@@ -100,7 +101,7 @@ void GraphComparer::processDifferingEntities(const EntityMismatchByName& sourceM
 
     for (const auto& match : matchingByName)
     {
-        auto& entityDiff = _result->differingEntities.emplace_back(ComparisonResult::EntityDifference
+        auto& entityDiff = result.differingEntities.emplace_back(ComparisonResult::EntityDifference
         {
             match.second.fingerPrint,
             match.second.node,
@@ -120,7 +121,7 @@ void GraphComparer::processDifferingEntities(const EntityMismatchByName& sourceM
 
     for (const auto& mismatch : missingInSource)
     {
-        _result->differingEntities.emplace_back(ComparisonResult::EntityDifference
+        result.differingEntities.emplace_back(ComparisonResult::EntityDifference
         {
             mismatch.second.fingerPrint,
             mismatch.second.node,
@@ -131,7 +132,7 @@ void GraphComparer::processDifferingEntities(const EntityMismatchByName& sourceM
 
     for (const auto& mismatch : missingInBase)
     {
-        _result->differingEntities.emplace_back(ComparisonResult::EntityDifference
+        result.differingEntities.emplace_back(ComparisonResult::EntityDifference
         {
             mismatch.second.fingerPrint,
             mismatch.second.node,

libs/scene/merge/GraphComparer.h

@@ -17,14 +17,12 @@ namespace scene
 namespace merge
 {
 
+/**
+ * Static utility class to compare two scenes given by their root nodes.
+ */
 class GraphComparer
 {
 private:
-    scene::IMapRootNodePtr _source;
-    scene::IMapRootNodePtr _base;
-
-    ComparisonResult::Ptr _result;
-
     using Fingerprints = std::map<std::string, scene::INodePtr>;
 
 public:
@@ -38,32 +36,23 @@ class GraphComparer
     using EntityMismatchByName = std::map<std::string, EntityMismatch>;
 
 public:
-    GraphComparer(const scene::IMapRootNodePtr& source, const scene::IMapRootNodePtr& base) :
-        _source(source),
-        _base(base),
-        _result(new ComparisonResult(_source, _base))
-    {}
-
-    void compare();
-
-    const ComparisonResult::Ptr& getResult() const
-    {
-        return _result;
-    }
+    // Compares the two graphs and returns the result
+    static ComparisonResult::Ptr Compare(const scene::IMapRootNodePtr& source, const scene::IMapRootNodePtr& base);
 
 private:
-    void processDifferingEntities(const EntityMismatchByName& sourceMismatches, const EntityMismatchByName& baseMismatches);
+    static void processDifferingEntities(ComparisonResult& result, const EntityMismatchByName& sourceMismatches, 
+        const EntityMismatchByName& baseMismatches);
 
-    Fingerprints collectEntityFingerprints(const scene::INodePtr& root);
-    Fingerprints collectPrimitiveFingerprints(const scene::INodePtr& parent);
+    static Fingerprints collectEntityFingerprints(const scene::INodePtr& root);
+    static Fingerprints collectPrimitiveFingerprints(const scene::INodePtr& parent);
 
-    Fingerprints collectNodeFingerprints(const scene::INodePtr& parent,
+    static Fingerprints collectNodeFingerprints(const scene::INodePtr& parent,
         const std::function<bool(const scene::INodePtr& node)>& nodePredicate);
 
-    std::list<ComparisonResult::KeyValueDifference> compareKeyValues(
+    static std::list<ComparisonResult::KeyValueDifference> compareKeyValues(
         const scene::INodePtr& sourceNode, const scene::INodePtr& baseNode);
 
-    std::list<ComparisonResult::PrimitiveDifference> compareChildNodes(
+    static std::list<ComparisonResult::PrimitiveDifference> compareChildNodes(
         const scene::INodePtr& sourceNode, const scene::INodePtr& baseNode);
 };
 

radiantcore/map/algorithm/Import.cpp

@@ -375,15 +375,11 @@ void importFromStream(std::istream& stream)
     }
 }
 
-scene::merge::ComparisonResult::Ptr compareGraphs(const scene::IMapRootNodePtr& source, const scene::IMapRootNodePtr& target)
+scene::merge::ComparisonResult::Ptr compareGraphs(const scene::IMapRootNodePtr& source, const scene::IMapRootNodePtr& base)
 {
-    assert(source && target);
+    assert(source && base);
 
-    scene::merge::GraphComparer comparer(source, target);
-
-    comparer.compare();
-
-    return comparer.getResult();
+    return scene::merge::GraphComparer::Compare(source, base);
 }
 
 }

radiantcore/map/algorithm/Import.h

@@ -56,8 +56,8 @@ MapFormatPtr determineMapFormat(std::istream& stream, const std::string& type);
   */
 MapFormatPtr determineMapFormat(std::istream& stream);
 
-// Runs a comparison of "source" (to be merged) against the "target" (merge target)
-scene::merge::ComparisonResult::Ptr compareGraphs(const scene::IMapRootNodePtr& source, const scene::IMapRootNodePtr& target);
+// Runs a comparison of "source" (changed map) against the "base" (original)
+scene::merge::ComparisonResult::Ptr compareGraphs(const scene::IMapRootNodePtr& source, const scene::IMapRootNodePtr& base);
 
 }
 

test/MapMerging.cpp

@@ -234,10 +234,7 @@ inline ComparisonResult::Ptr performComparison(const std::string& targetMap, con
     auto resource = GlobalMapResourceManager().createFromPath(sourceMapPath);
     EXPECT_TRUE(resource->load()) << "Test map not found in path " << sourceMapPath;
 
-    GraphComparer comparer(resource->getRootNode(), GlobalMapModule().getRoot());
-    comparer.compare();
-
-    return comparer.getResult();
+    return GraphComparer::Compare(resource->getRootNode(), GlobalMapModule().getRoot());
 }
 
 inline bool resultHasEntityDifference(const ComparisonResult::Ptr& result, const std::string& name,