Skip to content

Commit

Permalink
#5638: More test cases. Change group fingerprint calculation to only …
Browse files Browse the repository at this point in the history 
…look at an entity's name instead of its fingerprint.

For verifying links between entities a matching name is enough.
  • Loading branch information
@codereader
codereader committed Jun 4, 2021
1 parent a1bac31 commit a098155
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 16 deletions.
57 changes: 42 additions & 15 deletions libs/scene/merge/GraphComparer.cpp
View file
Expand Up @@ -21,8 +21,9 @@ namespace
{
inline std::string getEntityName(const INodePtr& node)
{
assert(node->getNodeType() == INode::Type::Entity);
auto entity = Node_getEntity(node);

return entity->isWorldspawn() ? "worldspawn" : entity->getKeyValue("name");
}
}
Expand Down Expand Up @@ -327,25 +328,43 @@ void GraphComparer::compareSelectionGroups(ComparisonResult& result)
compareSelectionGroups(result, matchingEntity.sourceNode, matchingEntity.baseNode);

// Each node of the matching source entity must have a counter-part in the base entity
auto sourcePrimitives = collectPrimitiveFingerprints(matchingEntity.sourceNode);
auto basePrimitives = collectPrimitiveFingerprints(matchingEntity.baseNode);
compareSelectionGroupsOfPrimitives(result, matchingEntity.sourceNode, matchingEntity.baseNode);
}

for (const auto& pair : sourcePrimitives)
// Compare mismatching entities that have a counterpart in the base map
for (const auto& mismatchingEntity : result.differingEntities)
{
// We only compare entities which are present in both maps
if (!mismatchingEntity.sourceNode || !mismatchingEntity.baseNode)
{
// Look up the counterart in the base map and compare
const auto& sourcePrimitive = pair.second;
auto counterpart = basePrimitives.find(pair.first);
continue;
}

if (counterpart != basePrimitives.end())
{
const auto& basePrimitive = counterpart->second;
compareSelectionGroups(result, mismatchingEntity.sourceNode, mismatchingEntity.baseNode);

compareSelectionGroups(result, sourcePrimitive, basePrimitive);
}
}
// Check each child of the mismatching source entity, it might have counter-parts in the base map
compareSelectionGroupsOfPrimitives(result, mismatchingEntity.sourceNode, mismatchingEntity.baseNode);
}
}

void GraphComparer::compareSelectionGroupsOfPrimitives(ComparisonResult& result, const INodePtr& sourceNode, const INodePtr& baseNode)
{
// Check each node of the mismatching source entity, it might have counter-parts in the base map
auto sourcePrimitives = collectPrimitiveFingerprints(sourceNode);
auto basePrimitives = collectPrimitiveFingerprints(baseNode);

for (const auto& pair : sourcePrimitives)
{
// Look up the counterart in the base map and compare
const auto& sourcePrimitive = pair.second;
auto counterpart = basePrimitives.find(pair.first);

// Compare mismatching source entities
if (counterpart != basePrimitives.end())
{
const auto& basePrimitive = counterpart->second;
compareSelectionGroups(result, sourcePrimitive, basePrimitive);
}
}
}

void GraphComparer::compareSelectionGroups(ComparisonResult& result, const INodePtr& sourceNode, const INodePtr& baseNode)
Expand Down Expand Up @@ -416,11 +435,19 @@ std::string GraphComparer::calculateGroupFingerprint(const selection::ISelection
std::vector<std::string> memberFingerprints(group->size());
group->foreachNode([&](const INodePtr& member)
{
if (member->getNodeType() == INode::Type::Entity)
{
// Group links between entities should use the entity's name to check for equivalence
// even if the entity has changed key values or primitives, the link is intact when the name is equal
memberFingerprints.emplace_back(getEntityName(member));
return;
}

auto comparable = std::dynamic_pointer_cast<IComparableNode>(member);

if (comparable)
{
memberFingerprints.emplace_back(comparable->getFingerprint());
memberFingerprints.emplace_back(comparable->getFingerprint());
}
});

Expand Down
1 change: 1 addition & 0 deletions libs/scene/merge/GraphComparer.h
View file
Expand Up @@ -57,6 +57,7 @@ class GraphComparer

static void compareSelectionGroups(ComparisonResult& result);
static void compareSelectionGroups(ComparisonResult& result, const INodePtr& sourceNode, const INodePtr& baseNode);
static void compareSelectionGroupsOfPrimitives(ComparisonResult& result, const INodePtr& sourceNode, const INodePtr& baseNode);
static std::string calculateGroupFingerprint(const selection::ISelectionGroupPtr& group);
};

Expand Down
48 changes: 47 additions & 1 deletion test/MapMerging.cpp
View file
Expand Up @@ -857,6 +857,52 @@ TEST_F(MapMergeTest, GroupMemberOrdering)
EXPECT_TRUE(result->selectionGroupDifferences.empty()) << "Group ordering shouldn't make a difference";
}

// Group links between entities should use the entity's name to check for equivalence
// even if the entity has changed key values or primitives, the link is intact when the name is equal
TEST_F(MapMergeTest, GroupLinksBetweenEntities)
{
auto originalResource = GlobalMapResourceManager().createFromPath("maps/merging_groups_1.mapx");
EXPECT_TRUE(originalResource->load()) << "Test map not found";

auto changedResource = GlobalMapResourceManager().createFromPath("maps/merging_groups_1.mapx");
EXPECT_TRUE(changedResource->load()) << "Test map not found";

auto result = GraphComparer::Compare(changedResource->getRootNode(), originalResource->getRootNode());
EXPECT_TRUE(result->selectionGroupDifferences.empty()) << "Unchanged resource should be the same as the original";

// Create a group out of two brushes and an entity
auto& originalGroupManager = originalResource->getRootNode()->getSelectionGroupManager();
auto originalGroup = originalGroupManager.createSelectionGroup();

// Find the two defined brushes
auto brush11 = algorithm::findFirstBrushWithMaterial(algorithm::findWorldspawn(originalResource->getRootNode()), "textures/numbers/11");
auto brush12 = algorithm::findFirstBrushWithMaterial(algorithm::findWorldspawn(originalResource->getRootNode()), "textures/numbers/12");
auto funcStatic = algorithm::getEntityByName(originalResource->getRootNode(), "expandable");

originalGroup->addNode(brush11);
originalGroup->addNode(brush12);
originalGroup->addNode(funcStatic);

// Do the same in the other map, but in a different order
auto& changedGroupManager = changedResource->getRootNode()->getSelectionGroupManager();
auto changedGroup = changedGroupManager.createSelectionGroup();

// Find the two defined brushes
brush11 = algorithm::findFirstBrushWithMaterial(algorithm::findWorldspawn(changedResource->getRootNode()), "textures/numbers/11");
brush12 = algorithm::findFirstBrushWithMaterial(algorithm::findWorldspawn(changedResource->getRootNode()), "textures/numbers/12");
funcStatic = algorithm::getEntityByName(changedResource->getRootNode(), "expandable");

// Change a key value on the entity to change its fingerprint
Node_getEntity(funcStatic)->setKeyValue("changedkey", "changedvalue");

changedGroup->addNode(brush11);
changedGroup->addNode(funcStatic);
changedGroup->addNode(brush12);

result = GraphComparer::Compare(changedResource->getRootNode(), originalResource->getRootNode());
EXPECT_TRUE(result->selectionGroupDifferences.empty()) << "Group ordering shouldn't make a difference and should only look at the names";
}

// Entity with no changed geometry, but with changed group membership in its children
TEST_F(MapMergeTest, GroupDifferenceInMatchingEntity)
{
Expand Down Expand Up @@ -1002,7 +1048,7 @@ TEST_F(MapMergeTest, GroupDifferenceOfMismatchingEntity)
Node_getEntity(funcStatic)->setKeyValue("dummyvalue", "changed");

// Add this func_static into a new group together with brush 11
auto brush11 = algorithm::findFirstBrushWithMaterial(algorithm::findWorldspawn(originalResource->getRootNode()), "textures/numbers/11");
auto brush11 = algorithm::findFirstBrushWithMaterial(algorithm::findWorldspawn(changedResource->getRootNode()), "textures/numbers/11");

changedGroup->addNode(funcStatic);
changedGroup->addNode(brush11);
Expand Down

0 comments on commit a098155

Please sign in to comment.