Null pointer on connecting nodes in certain examples

[chateauferret]

In some of the examples, e.g. the calculator, there is a crash because of a null ProxyWidget pointer in places like this in NodeGraphicsObject.cpp line 81:

void NodeGraphicsObject::updateQWidgetEmbedPos()
{
_proxyWidget->setPos(nodeScene()->nodeGeometry().widgetPosition(_nodeId));
}

I might try and fix that by simply testing the pointer for NULL before it is dereferenced, but thought it wise to consult you first.

[chateauferret]

That's on Qt 6.7.2 in Microsoft Windows (and seen on two different computers btw).

[chateauferret]

OK, so it seems to behave if I just make sure the ProxyWidget pointer isn't NULL before I try to dereference it. There are a few other places where I think this is a risk, because some of the other examples are crashing at similar but different places. So I'll go through and look for them then see if I can put down a pull request.

Root cause seems to be line 91, in the method embedQWidget where _graphModel.nodeData must be returning NULL.

Think we can fix this without too much bother then.

[chateauferret]

Slightly better, let's suggest this embedQWidget - always assign a value to the pointer.

void NodeGraphicsObject::embedQWidget()
{
    AbstractNodeGeometry &geometry = nodeScene()->nodeGeometry();
    geometry.recomputeSize(_nodeId);
    _proxyWidget = new QGraphicsProxyWidget(this);
    if (auto w = _graphModel.nodeData(_nodeId, NodeRole::Widget).value<QWidget *>()) {
        _proxyWidget->setWidget(w);
        ....

[Daguerreo]

Slightly better, let's suggest this embedQWidget - always assign a value to the pointer.

void NodeGraphicsObject::embedQWidget()
{
    AbstractNodeGeometry &geometry = nodeScene()->nodeGeometry();
    geometry.recomputeSize(_nodeId);
    _proxyWidget = new QGraphicsProxyWidget(this);
    if (auto w = _graphModel.nodeData(_nodeId, NodeRole::Widget).value<QWidget *>()) {
        _proxyWidget->setWidget(w);
        ....

If embedQWidget() is called multiple time you continue to allocate a new QGraphicsProxyWidget. It should be included a guard at least to destroy the previous pointer.

[chateauferret]

void NodeGraphicsObject::embedQWidget()
{
    AbstractNodeGeometry &geometry = nodeScene()->nodeGeometry();
    geometry.recomputeSize(_nodeId);
    if (! _proxyWidget) {
        _proxyWidget = new QGraphicsProxyWidget(this);
    }
    if (auto w = _graphModel.nodeData(_nodeId, NodeRole::Widget).value<QWidget *>()) {
        _proxyWidget->setWidget(w);
        ....

[chateauferret]

Slightly better, let's suggest this embedQWidget - always assign a value to the pointer.

void NodeGraphicsObject::embedQWidget()
{
    AbstractNodeGeometry &geometry = nodeScene()->nodeGeometry();
    geometry.recomputeSize(_nodeId);
    _proxyWidget = new QGraphicsProxyWidget(this);
    if (auto w = _graphModel.nodeData(_nodeId, NodeRole::Widget).value<QWidget *>()) {
        _proxyWidget->setWidget(w);
        ....

If embedQWidget() is called multiple time you continue to allocate a new QGraphicsProxyWidget. It should be included a guard at least to destroy the previous pointer.

Good point. Fixed.

[paceholder]

Thanks for addressing it. It's been recently added in 77f3ad1 .
I am not sure if we need to create a proxy wiged for no purpose if there is no user's widget to embed.
Maybe it is enough to check for nullptr in the problematic function NodeGraphicsObject::updateQWidgetEmbedPos()