Dynamic Ports

[Daguerreo]

There are several try of implementation of dynamic ports. The current status is:

paceholder#209, paceholder#212

• Deal add and remove only of the last port. Removing any port but last break the layout
• Loading of dynamic ports is not supported.

paceholder#251 is an improvement to 202 and 219, yet not completed because do not handle remotion of middle ports.

• Missing sample
• Loading of dynamic ports is not supported.

paceholder#268 try to make a patch to the dynamic loading of 209.

However the best implementation has not a pull request and it's from @matthew-reid dynamicInterface branch

• It correctly send port add and remove signal with port index
• It correctly shift the ports if a middle input is removed.
• Does not declare friend Node class as the previous.

However the issues with that branch are:

• It's from 2018 and it's quite behind the current master branch.
• Code is mixed with another feature.
• Do not support loading.
• Do not provide examples.

---

Based on those previous attempt I've made a branch dynamic-ports with these features:

• Send port index in adding and removing.
• Support port shifting if middle port is removed.
• Support save and loading.
• Add example models in calculator example.

To deal with dynamic loading/saving I added a method hasDynamicPorts(PortType) const to NodeDataModel. If a model is marked as dynamic for port(s) type, when serializing it automatically add an entry dynamic_inputs and/or dynamic_outputs to write the number of ports at save time because they can be different from when they are created.

2020-12-20-17-39-01.mp4

[matthew-reid]

This looks good, I'm glad you were able to improve on my attempt. I'd like to see this merged if the load issue can be fixed.

[Daguerreo]

This looks good, I'm glad you were able to improve on my attempt. I'd like to see this merged if the load issue can be fixed.

thank you @matthew-reid. If at any moment you could look at the code I'd be glad, 4 eyes are better of two.

[Daguerreo]

The bug about loading has been fixed, now feature should be complete.

[matthew-reid]

The code looks good. The only thing I would suggest is to add static helper functions readNumInputs(obj), readNumOutputs(obj) to NodeDataModel to parse the number of ports from the json object, so that the implementation of restore() doesn't need to know about the "dynamic_inputs" and "dynamic_outputs" string. An example restore override would look like:

void MaxModel::restore(const QJsonObject& obj)
{
   int in = readNumInputs(obj);
   if(in > 0){
     // since when node is saved port's number is size+1 with an empty port
     // to restore the correct size of the array it has to be input-1
      _numberList.resize(in-1);
   }
}

[Daguerreo]

I agree with your idea, yet don't know how to put it down without complicate the library or doing something very situational. Maybe a whole helper class for serialized information could be useful since there are several hardcoded key string. Need to think a bit about that.

[Sarrasor]

Hello, @Daguerreo. Cool feature!

I'm trying to implement dynamic outputs, and have faced several issues.

To add outputs dynamically, I have implemented two slots:

void MyNodeDataModel:outputConnectionCreated(Connection const& c)
{
  int out_port_index = c.getPortIndex(QtNodes::PortType::Out);
  if (out_port_index == static_cast<int>(_children_list.size()))
  {
    _children_list.push_back(c.getNode(QtNodes::PortType::In)->nodeDataModel()->name());
    Q_EMIT portAdded(PortType::Out, _children_list.size());
  }
  else
  {
    _children_list[out_port_index] = c.getNode(QtNodes::PortType::In)->nodeDataModel()->name(); 
  }
}

void MyNodeDataModel::outputConnectionDeleted(Connection const& c)
{
  int out_port_index = c.getPortIndex(QtNodes::PortType::Out);
  int in_port_index = c.getPortIndex(QtNodes::PortType::In);
  if((out_port_index != -1) && (in_port_index != -1))
  {
    if (out_port_index < static_cast<int>(_children_list.size()))
    {
      _children_list.erase(_children_list.begin() + out_port_index);
      Q_EMIT portRemoved(PortType::Out, out_port_index);
    }
  }
}

The issue happens on delete. MyNodeDataModel::portRemoved() triggers Node::onPortRemoved(), which will call Node::connectionRemoved(), and since we have already removed current connection, a segfault will occur. The way to solve the problem is to add current connection check:

for (Connection* connection : connections)
{
  int out_port_index = connection->getPortIndex(QtNodes::PortType::Out);
  int in_port_index = connection->getPortIndex(QtNodes::PortType::In);
  if ((portType == PortType::In) && (in_port_index == index))
  {
    continue;
  }
  if ((portType == PortType::Out) && (out_port_index == index))
  {
    continue;
  }
  Q_EMIT connectionRemoved(*connection);
}

However, there is still a problem with consequent connections. If I remove a child node - everything is file. If I just click on in port of a child node, the app will remove the connection and will try to add it again. At the same time, the app will shift other nodes, which will lead to the following issue:

dynamic_outputs.mp4

Maybe I do not understand something. Is there a better way to handle dynamic outputs?

[Daguerreo]

Hello @Sarrasor. Calculator example do contains an example node with dynamic ports. Have you looked at it?

[Sarrasor]

Hello @Sarrasor. Calculator example do contains an example node with dynamic ports. Have you looked at it?

Yep. The thing is - the example considers dynamic input ports, not dynamic output ports.

For example, consider the MaxModel from the calculator example

The logic of the dynamic port increments and signals is implemented in the MaxModel:: setInData(). In my case, however, I want to have dynamic outputs. Since there is no setOutData method, I have implemented the logic in slots

Is there an example of dynamic output ports, which I missed?

[Daguerreo]

I'm going to take a look into it. I could not do it in a very short time though. At first glance seems you're going to remove the port when the connection does still exists in the graphics object.

[Daguerreo]

@Sarrasor I've noticed NodeDataModel::outputConnectionDeleted() and NodeDataModel::ConnectionDeleted() are called twice when a node is deleted. There are a few of double call in the library, this one could affect the behaviour. I'm going to investigate

[Daguerreo]

Ok, I've should fixed the multiple delete connection call and added some nullptr check. In addition, I added a node using dynamic outputs in calculator example

dynoutput-.5.mp4

[Sarrasor]

Great, thank you!

Now my code works too:

example.mp4

The only thing I can suggest is to explicitly cast entry.size() to int. Yes, there is a non-negativity check before, but just to be safe.

NodeEditor/src/NodeState.cpp

Line 76 in 69149b3

if(portIndex >= 0 && portIndex < entry.size()){

if(portIndex >= 0 && portIndex < static_cast<int>(entry.size()))

[RValner]

EDIT: I just realized that this issue thread is on a fork of nodeeditor. Opened a new issue here

@Daguerreo I'm trying to do something very similar as shown in the video above, i.e., whenever an input connection is created, +1 input port is added. I've seen the dynamic_ports example, but it's built on top of AbstractGraphModel, which to me seemed unnecessarily complex. Thus decided to use NodeDelegateModel instead, keeping the code on node level.

The issue i'm having is that if I have for example 3 connections and then remove the middle one, the connections will not be shifted, as shown in the video below:

dynamic_ports_bug_1.mp4

If I add portsAboutToBeDeleted and portsDeleted calls (as instructed here) to the overridden inputConnectionDeleted method:

void UmrfNodeModel::inputConnectionDeleted(QtNodes::ConnectionId const &ci)
{
    std::cout << ci.inPortIndex << std::endl;
    portsAboutToBeDeleted(PortType::In, ci.inPortIndex, ci.inPortIndex);
    input_connections_.erase(input_connections_.begin() + ci.inPortIndex);
    portsDeleted();
}

then deleting any connection above other connections will result removing all connections below it, as shown in the video:

dynamic_ports_bug_2.mp4

The std::cout << ci.inPortIndex << std::endl; also shows that inputConnectionDeleted is indeed called for each deleted connection, hinting a recursive behavior. My gut feeling says i've added portsAboutToBeDeleted and portsDeleted to a wrong place, but atm I'm out of ideas, as there is not much to play with in the NodeDelegateModel API. I did skim throuh one of the older commits that implemented the desired behavior, but the API seems to have changed quite a bit in v3.

Any help is much appreciated.