Skip to content

Commit

Permalink
#5639: Fix node removal and addition algorithm in LayerMerger.
Browse files Browse the repository at this point in the history
We need to add nodes to layers first, since trying to remove a node from its last layer will inevitably put it on Default.
  • Loading branch information
codereader committed Jun 7, 2021
1 parent 65c489c commit 8e64384
Showing 1 changed file with 59 additions and 20 deletions.
79 changes: 59 additions & 20 deletions libs/scene/merge/LayerMerger.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,21 @@ class LayerMerger
scene::ILayerManager& _sourceManager;
scene::ILayerManager& _baseManager;

std::stringstream _log;
std::vector<Change> _changes;

private:
// Working members that is only needed during processing

std::map<std::string, INodePtr> _sourceNodes;
std::map<std::string, INodePtr> _baseNodes;

std::vector<std::string> _baseLayerNamesToRemove;

std::stringstream _log;
std::vector<Change> _changes;
// The remove-node-from-layer action that needs to be executed
std::vector<std::pair<int, INodePtr>> _baseNodesToRemoveFromLayers;
// The add-node-to-layer action that needs to be executed
std::vector<std::pair<int, INodePtr>> _baseNodesToAddToLayers;

public:

Expand Down Expand Up @@ -88,6 +96,10 @@ class LayerMerger

void adjustBaseLayers()
{
cleanupWorkingData();
_changes.clear();
_log.str(std::string());

// Collect all source and base nodes for easier lookup
_sourceRoot->foreachNode([&](const INodePtr& node)
{
Expand Down Expand Up @@ -116,6 +128,36 @@ class LayerMerger
_sourceManager.foreachLayer(
std::bind(&LayerMerger::processSourceLayer, this, std::placeholders::_1, std::placeholders::_2));

// Execute the actions we found during source layer processing
// Add to new layers first, since removing a node from its last layer might not succeed otherwise
for (const auto& pair : _baseNodesToAddToLayers)
{
_log << "Adding node " << pair.second->name() << " to layer " << pair.first << std::endl;

pair.second->addToLayer(pair.first);

_changes.emplace_back(Change
{
pair.first,
pair.second,
Change::Type::NodeAddedToLayer
});
}

for (const auto& pair : _baseNodesToRemoveFromLayers)
{
_log << "Removing node " << pair.second->name() << " from layer " << pair.first << std::endl;

pair.second->removeFromLayer(pair.first);

_changes.emplace_back(Change
{
pair.first,
pair.second,
Change::Type::NodeRemovedFromLayer
});
}

_log << "Removing " << _baseLayerNamesToRemove.size() << " base layers that have been marked for removal." << std::endl;

// Remove all base layers that are no longer necessary
Expand All @@ -133,9 +175,20 @@ class LayerMerger
Change::Type::BaseLayerRemoved
});
}

cleanupWorkingData();
}

private:
void cleanupWorkingData()
{
_sourceNodes.clear();
_baseNodes.clear();
_baseLayerNamesToRemove.clear();
_baseNodesToRemoveFromLayers.clear();
_baseNodesToAddToLayers.clear();
}

using LayerMembers = std::map<std::string, scene::INodePtr>;

void processBaseLayer(int baseLayerId, const std::string& baseLayerName)
Expand Down Expand Up @@ -268,15 +321,8 @@ class LayerMerger
continue;
}

_log << "Removing node " << baseNode->second->name() << " from layer " << sourceLayerName << std::endl;
baseNode->second->removeFromLayer(baseLayerId);

_changes.emplace_back(Change
{
baseLayerId,
baseNode->second,
Change::Type::NodeRemovedFromLayer
});
_log << "Marking node " << baseNode->second->name() << " for removal from layer " << sourceLayerName << std::endl;
_baseNodesToRemoveFromLayers.emplace_back(std::make_pair(baseLayerId, baseNode->second));
}

for (const auto& pair : membersToBeAdded)
Expand All @@ -290,15 +336,8 @@ class LayerMerger
continue;
}

_log << "Adding node " << baseNode->second->name() << " to layer " << sourceLayerName << std::endl;
baseNode->second->addToLayer(baseLayerId);

_changes.emplace_back(Change
{
baseLayerId,
baseNode->second,
Change::Type::NodeAddedToLayer
});
_log << "Marking node " << baseNode->second->name() << " for addition to layer " << sourceLayerName << std::endl;
_baseNodesToAddToLayers.emplace_back(std::make_pair(baseLayerId, baseNode->second));
}
}

Expand Down

0 comments on commit 8e64384

Please sign in to comment.