Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow node name reuse after delete #733

Merged
merged 2 commits into from
Dec 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 14 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,20 @@
# History


## Version 2.4.3

### Known issues

- Crash when closing a project window on macOS 12+ (Qt4 only). #712
- Rendering sometimes silently stalls after X frames. #248
- Some image formats may have issues (PCX, PSB). #602
- MTS video files are sometimes not read correctly. #186

### Changes

- Allow creating a node with the same name that was just deleted. #732


## Version 2.4.2

### Known issues
Expand Down
12 changes: 6 additions & 6 deletions Engine/NodeGroup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ NodeCollection::checkNodeName(const Node* node,
foundNodeWithName = false;
QMutexLocker l(&_imp->nodesMutex);
for (NodesList::iterator it = _imp->nodes.begin(); it != _imp->nodes.end(); ++it) {
if ( (it->get() != node) && ( (*it)->getScriptName_mt_safe() == *nodeName ) ) {
if ( (it->get() != node) && (*it)->isActivated() && ( (*it)->getScriptName_mt_safe() == *nodeName ) ) {
foundNodeWithName = true;
break;
}
Expand Down Expand Up @@ -760,7 +760,7 @@ NodeCollectionPrivate::findNodeInternal(const std::string& name,
QMutexLocker k(&nodesMutex);

for (NodesList::const_iterator it = nodes.begin(); it != nodes.end(); ++it) {
if ( (*it)->getScriptName_mt_safe() == name ) {
if ( (*it)->isActivated() && (*it)->getScriptName_mt_safe() == name ) {
if ( !recurseName.empty() ) {
NodeGroup* isGrp = (*it)->isEffectGroup();
if (isGrp) {
Expand All @@ -769,7 +769,7 @@ NodeCollectionPrivate::findNodeInternal(const std::string& name,
NodesList children;
(*it)->getChildrenMultiInstance(&children);
for (NodesList::iterator it2 = children.begin(); it2 != children.end(); ++it2) {
if ( (*it2)->getScriptName_mt_safe() == recurseName ) {
if ( (*it2)->isActivated() && (*it2)->getScriptName_mt_safe() == recurseName ) {
return *it2;
}
}
Expand Down Expand Up @@ -924,7 +924,7 @@ NodeCollection::checkIfNodeLabelExists(const std::string & n,
QMutexLocker k(&_imp->nodesMutex);

for (NodesList::const_iterator it = _imp->nodes.begin(); it != _imp->nodes.end(); ++it) {
if ( (it->get() != caller) && ( (*it)->getLabel_mt_safe() == n ) ) {
if ( (it->get() != caller) && (*it)->isActivated() && ( (*it)->getLabel_mt_safe() == n ) ) {
return true;
}
}
Expand All @@ -939,7 +939,7 @@ NodeCollection::checkIfNodeNameExists(const std::string & n,
QMutexLocker k(&_imp->nodesMutex);

for (NodesList::const_iterator it = _imp->nodes.begin(); it != _imp->nodes.end(); ++it) {
if ( (it->get() != caller) && ( (*it)->getScriptName_mt_safe() == n ) ) {
if ( (it->get() != caller) && (*it)->isActivated() && ( (*it)->getScriptName_mt_safe() == n ) ) {
return true;
}
}
Expand Down Expand Up @@ -1165,7 +1165,7 @@ NodeGroup::getInputLabel(int inputNb) const
///If the input name starts with "input" remove it, otherwise keep the full name

input = _imp->inputs[inputNb].lock();
if (!input) {
if (!input || !input->isActivated()) {
return std::string();
}
}
Expand Down
2 changes: 1 addition & 1 deletion Engine/NodeInputs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1735,7 +1735,7 @@ Node::hasSequentialOnlyNodeUpstream(std::string & nodeName) const

for (InputsV::iterator it = _imp->inputs.begin(); it != _imp->inputs.end(); ++it) {
NodePtr input = it->lock();
if ( input && input->hasSequentialOnlyNodeUpstream(nodeName) && input->getEffectInstance()->isWriter() ) {
if ( input && input->isActivated() && input->hasSequentialOnlyNodeUpstream(nodeName) && input->getEffectInstance()->isWriter() ) {
nodeName = input->getScriptName_mt_safe();

return true;
Expand Down
33 changes: 24 additions & 9 deletions Engine/NodeName.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,19 @@

NATRON_NAMESPACE_ENTER

// Trim underscore followed by digits at the end of baseName (see #732).
static
void trimNumber(std::string &baseName)
{
std::size_t found_underscore = baseName.rfind('_');
if (found_underscore != std::string::npos && found_underscore != (baseName.size() - 1)) {
std::size_t found_nondigit = baseName.find_last_not_of("0123456789");
if (found_nondigit == found_underscore) {
baseName.erase(found_underscore);
}
}
}

void
Node::initNodeScriptName(const NodeSerialization* serialization, const QString& fixedName)
{
Expand All @@ -51,15 +64,16 @@ Node::initNodeScriptName(const NodeSerialization* serialization, const QString&
if (!fixedName.isEmpty()) {

std::string baseName = fixedName.toStdString();
std::string name = baseName;
trimNumber(baseName);
std::string name;
int no = 1;
do {
name = baseName;
if (no > 1) {
name += '_';
std::stringstream ss;
ss << baseName;
ss << '_';
ss << no;
name = ss.str();
name += ss.str();
}
++no;
} while ( group && group->checkIfNodeNameExists(name, this) );
Expand All @@ -72,16 +86,17 @@ Node::initNodeScriptName(const NodeSerialization* serialization, const QString&
QMutexLocker k(&_imp->nameMutex);
_imp->cacheID = serialization->getCacheID();
}
const std::string& baseName = serialization->getNodeScriptName();
std::string name = baseName;
std::string baseName = serialization->getNodeScriptName();
trimNumber(baseName);
std::string name;
int no = 1;
do {
name = baseName;
if (no > 1) {
name += '_';
std::stringstream ss;
ss << baseName;
ss << '_';
ss << no;
name = ss.str();
name += ss.str();
}
++no;
} while ( group && group->checkIfNodeNameExists(name, this) );
Expand Down
27 changes: 21 additions & 6 deletions Gui/NodeGraphPrivate10.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,19 @@ NodeGraphPrivate::pasteNodesInternal(const NodeClipBoard & clipboard,
}
} // pasteNodesInternal

// Trim underscore followed by digits at the end of baseName (see #732).
static
void trimNumber(std::string &baseName)
{
std::size_t found_underscore = baseName.rfind('_');
if (found_underscore != std::string::npos && found_underscore != (baseName.size() - 1)) {
std::size_t found_nondigit = baseName.find_last_not_of("0123456789");
if (found_nondigit == found_underscore) {
baseName.erase(found_underscore);
}
}
}

NodeGuiPtr
NodeGraphPrivate::pasteNode(const NodeSerializationPtr & internalSerialization,
const NodeGuiSerializationPtr & guiSerialization,
Expand Down Expand Up @@ -179,20 +192,22 @@ NodeGraphPrivate::pasteNode(const NodeSerializationPtr & internalSerialization,
std::string name;
if ( ( grp == group.lock() ) && ( !internalSerialization->getNode() || ( internalSerialization->getNode()->getGroup() == group.lock() ) ) ) {
//We pasted the node in the same group, give it another label
std::string baseName = internalSerialization->getNodeLabel();
trimNumber(baseName);
std::string name;
int no = 1;
std::string label = internalSerialization->getNodeLabel();
do {
name = baseName;
if (no > 1) {
name += '_';
std::stringstream ss;
ss << internalSerialization->getNodeLabel();
ss << '_';
ss << no;
label = ss.str();
name += ss.str();
}
++no;
} while ( grp->checkIfNodeLabelExists( label, n.get() ) );
} while ( grp->checkIfNodeLabelExists( name, n.get() ) );

n->setLabel(label);
n->setLabel(name);
} else {
//If we paste the node in a different graph, it can keep its scriptname/label
}
Expand Down
14 changes: 12 additions & 2 deletions Gui/NodeGraphUndoRedo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1605,8 +1605,13 @@ GroupFromSelectionCommand::redo()
int inputNb = 0;
for (std::list<Project::NodesTree>::iterator it = trees.begin(); it != trees.end(); ++it) {
NodeGuiPtr foundOriginalOutputNode;
if (!it->output.node->isActivated()) {
continue;
}
const std::string outputNodeName = it->output.node->getScriptName_mt_safe();
for (NodesGuiList::const_iterator it3 = originalNodes.begin(); it3 != originalNodes.end(); ++it3) {
if ( (*it3)->getNode()->getScriptName_mt_safe() == it->output.node->getScriptName_mt_safe() ) {
NodePtr n = (*it3)->getNode();
if ( n && n->isActivated() && n->getScriptName_mt_safe() == outputNodeName) {
foundOriginalOutputNode = *it3;
break;
}
Expand All @@ -1621,8 +1626,13 @@ GroupFromSelectionCommand::redo()
for (std::list<Project::TreeInput>::iterator it2 = it->inputs.begin(); it2 != it->inputs.end(); ++it2) {
// Find the equivalent node in the original nodes and see which inputs we need to create
NodeGuiPtr foundOriginalNode;
if (!it2->node->isActivated()) {
continue;
}
const std::string originalNodeName = it2->node->getScriptName_mt_safe();
for (NodesGuiList::const_iterator it3 = originalNodes.begin(); it3 != originalNodes.end(); ++it3) {
if ( (*it3)->getNode()->getScriptName_mt_safe() == it2->node->getScriptName_mt_safe() ) {
NodePtr n = (*it3)->getNode();
if ( n && n->isActivated() && n->getScriptName_mt_safe() == originalNodeName) {
foundOriginalNode = *it3;
break;
}
Expand Down