nodeeditor

nodeeditor copied to clipboard

When an operation changes the number of ports. one can emit the new signal portCountChanged() from a NodeDataModel to trigger an update of the node.

Related issues: This is an alternative to #209 and #212. I had not noticed that there were already PRs for this before implementing it fo another project.

Key differences with #209: We handle the removal of connections. This implementation does not require to add a friend class Node. We use a single signal instead of both portAdded and portRemoved.

Limitations: As noticed in the comments, the removal of connections is not perfect. It assumes that the removed ports are always the last ones, so if it is not true and port types differe, this may lead to connections between ports of different types. In the RiverList example though there would be no problem. To address this, the signal should be replaced by two different signals portAdded and portRemoved taking as arguments the index of the added/removed ports.

This also includes somes static_casts to prevent msvc from raising some warnings. I can remove them if needed, as well as reduce the comments. Also, should I add an example like the river list of #209? Or some kind of unit test?

@paceholder CI scripts don't find Qt apparently; I don't think their failure is due to my code.

Works for me, thanks!

This works for me, however in my setup I get a segfault in the following method if I try to connect an input connection to nothing. Just click an input port and drag out to the left a bit and unclick.

NodeState::ConnectionPtrSet
NodeState::
connections(PortType portType, PortIndex portIndex) const
{
  auto const &connections = getEntries(portType);

  return connections[portIndex];
}

If I check connections for size and return an empty NodeState::ConnectionPtrSet in the case where it is too small then it works for me. This said, I don't see this as an appropriate fix. It seems like a hack as connections should probably always be the right size when getting the portIndex item from it. So something seems slightly amiss. It appears as though whatever is calling this with the port index doesn't have up to date information on the length of connections.

If I check connections for size and return an empty NodeState::ConnectionPtrSet in the case where it is too small then it works for me. This said, I don't see this as an appropriate fix. ...

It has been a while, I had to make additional changes to fix some edge cases.

The main thing I found was FlowScene::createConnection(), calls onDataUpdated() before connectionCreated(). Which is a problem because setInData() is called with the old (maybe illegal) connection state, which seems to be the main place it would need to be examined. So I changed that order.

IMHO the unchecked array accesses should be removed and throw exceptions, I've already added a bunch in my copy to get this to work.

There are a lot of cases where this can happen when using the API, not only from counting port numbers, but also due to complex control flow resulting from signals/slots or graph traversal. And the resulting heisenbugs are difficult to pinpoint.

All fair points. I am just beginning to wrap my head around how it is working as well. Your changes are a nice step in the right direction. I also added this function to NodeGraphicsObject (based on mouseMoveEvent since emitting that when resizing had the desired effect but I don't want resizable graphics items.

void NodeGraphicsObject::
updateSize()
{
  auto & geom  = _node.nodeGeometry();
  auto & state = _node.nodeState();

  if (auto w = _node.nodeDataModel()->embeddedWidget())
  {
    QSize oldSize = w->size();

    oldSize += QSize(1, 1);

    w->setFixedSize(oldSize);

    _proxyWidget->setMinimumSize(oldSize);
    _proxyWidget->setMaximumSize(oldSize);
    _proxyWidget->setPos(geom.widgetPosition());

    geom.recalculateSize();
  }

  QRectF r = scene()->sceneRect();

  r = r.united(mapToScene(boundingRect()).boundingRect());

  scene()->setSceneRect(r);
}

I can call these and end up with the node with the correct size to fit the embedded widget and show the dynamic port names properly.

      node.nodeState().updatePortCount(size(), 1);
      emit node.onPortCountChanged();
      node.nodeGraphicsObject().updateSize();

To shed some light on my use-case. I am not using the context system to add nodes to the graph. I am adding them with custom signals from other widgets. To make this even more fun, all the node connections are coming from a database, so I'm only really using the graph for display and minimal modification.

In any case, this may be some help to you. I know your changes are helpful to me.

Dynamic ports have been implemented. Refer to documentation and examples. The new code is based on an AbstractGraphModel (a.k.a. "headless" model). Thanks for the multiple contributions that paved the way to the latest version.