Skip to content

Commit

Permalink
Allow node name reuse after delete (#733)
Browse files Browse the repository at this point in the history 
* Allow node name reuse after delete

fixes #732

* when creating/pasting nodes, don't add more underscores than necessary.

addresses #732 (comment)
  • Loading branch information
@devernay
devernay committed Dec 24, 2021
1 parent b619297 commit ec9b55a
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 24 deletions.
14 changes: 14 additions & 0 deletions CHANGELOG.md
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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

1 comment on commit ec9b55a

@bonalex01dev
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very welcome! thanks Fred

Please sign in to comment.