Connection signals for dynamically created widgets

Deneguil

Hello,

I am making a graphical automata editor. I have a custom graphics view that I use as the canvas for adding the nodes and later on the edges. I want to display the parameters of the clicked node in the sidebar. To do so, I thought of having the node widgets emit a clicked signal with a pointer to the underlying information. That signal would them be caught by a StateParams slot in the main application class.

The relevant classes are as such :

StateWidget.h

constexpr float R = 50;

class StateWidget : public QGraphicsEllipseItem {
Q_OBJECT
    public:
        float x, y;
        State* m_State;

        inline StateWidget(QPointF p, QGraphicsItem* parent = nullptr): QGraphicsEllipseItem(p.x()-R/2,p.y()-R/2,R,R)
        {
            setBrush(QBrush(Qt::white));
            setZValue(10);
        }

    protected:
        inline void mousePressEvent(QGraphicsSceneMouseEvent *event) override {
            event->accept();
            std::cout << "State " << m_State->m_ID << " pressed\n";
            emit clicked(m_State);
        }
    
    signals:
        void clicked(State* s);
};

AutomatLab.h

class AutomatLab : public QMainWindow {
    Q_OBJECT

private:
    Ui::AutomatLab m_UI;
    GraphicView m_GV;
    Automata m_Automata;

    void ToggleBtn(QPushButton* btn, bool toggled);

public:
    static EClickState s_State;

    AutomatLab();
    ~AutomatLab();

public slots:
    void StateParams(State* s);
    void InsertStateMode(bool toggled);
    void InsertTransitionMode(bool toggled);
    void ButtonUsed();
};

The StateWidgets are instanciated in the graphics view through the list of states the automata has. It's a simple for each that creates a StateWidget on the heap, saves its reference in a vector so that it's deallocated properly when destroying the graphics view and adds it to the scene. I know it's quite inefficient as everything is rerendered every time a modification is done but I haven't managed to think of another way to so it yet.

void GraphicView::mousePressEvent(QMouseEvent* event) {
    if(itemAt(event->pos()) != nullptr) {
        QGraphicsView::mousePressEvent(event);
        return;
    }

    std::cout << "APP MODE : " << AutomatLab::s_State << std::endl;
    
    switch(AutomatLab::s_State) {
        case None: break;
        case Node: {
            a->AddState(EStateKind::None, event->pos().x(), event->pos().y()); 
            Render();   // This rerenders the entire canva on every new State :xdd:
        }
        case Transition: break;
    }

    emit clicked(event->pos());
}


void GraphicView::Render() {
    for(auto& s : a->m_States) {
        StateWidget* sw = new StateWidget(mapToScene(s.x, s.y));
        sw->m_State = &s;
        connect(sw, &StateWidget::clicked, &AutomatLab::StateParams);
        items.push_back(sw);
        scene->addItem(sw);
    }
}

The issue is that when I try to build I get this error

candidate function [with Func1 = void (StateWidget::*)(Automatlab::State *), Func2 = void (Automatlab::AutomatLab::*)(Automatlab::State *)] not viable: no known conversion from 'const typename QtPrivate::FunctionPointer<void (StateWidget::*)(State *)>::Object *' (aka 'const StateWidget *') to 'const typename QtPrivate::ContextTypeForFunctor<void (AutomatLab::*)(State *)>::ContextType *' (aka 'const Automatlab::AutomatLab *') for 3rd argument

When I've double checked that the overload exists for connect.

How can I achieve what I'm looking for in a clean way? I've thought of making the StateParams function static as there'd only be one instance of the UI at a time but I'm not a fan of that

Christian Ehrlicher

@Deneguil said in Connection signals for dynamically created widgets:

connect(sw, &StateWidget::clicked, &AutomatLab::StateParams);

You should pass a receiver object as third argument

Deneguil

@Christian-Ehrlicher said in Connection signals for dynamically created widgets:

You should pass a receiver object as third argument

Normally I would, but this overload was proposed by the LSP and it looked so convenient that I wanted to use it. I'll add a reference to the main app class to the graphics view then

JonB

@Deneguil said in Connection signals for dynamically created widgets:

When I've double checked that the overload exists for connect.

Normally I would, but this overload was proposed by the LSP and it looked so convenient that I wanted to use it.

[Btw, what's "LSP"?] As @Christian-Ehrlicher says for the solution. But OOI what overload of connect() do you think your call matches? If you think it's
template <typename PointerToMemberFunction, typename Functor> QMetaObject::Connection QObject::connect(const QObject *sender, PointerToMemberFunction signal, Functor functor)
that does not match on &AutomatLab::StateParams for Functor functor.

Deneguil

@JonB said in Connection signals for dynamically created widgets:

[Btw, what's "LSP"?]

LSP stands for Language Server Protocol, it's basically intellisense it tells you what functions are available and can detect some simple syntax errors before building

As @Christian-Ehrlicher says for the solution. But OOI what overload of connect() do you think your call matches? If you think it's
template <typename PointerToMemberFunction, typename Functor> QMetaObject::Connection QObject::connect(const QObject *sender, PointerToMemberFunction signal, Functor functor)
that does not match on &AutomatLab::StateParams for Functor functor.

Oh good catch, I thought it was saying Function instead.

I'm unable to pass a reference to my AutomatLab class to GraphicsView as it'd make for a circular dependency so I've been trying a workaround by having another signal and basically forwarding the signal to the AutomatLab class as such

void GraphicView::Render() {
    for(auto& s : a->m_States) {
        StateWidget* sw = new StateWidget(mapToScene(s.x, s.y));
        sw->m_State = &s;
        emit connectStateWidget(sw);
        items.push_back(sw);
        scene->addItem(sw);
    }
}

// AutomatLab.cpp constructor
connect(&m_GV, &GraphicView::connectStateWidget, this, [&] (StateWidget* sw) {
    connect(sw, &StateWidget::clicked, this, &AutomatLab::StateParams);
});

But now I'm getting a different error being

error: no member named 'staticMetaObject' in 'QGraphicsEllipseItem'; did you mean simply 'staticMetaObject'?

Even though my StateWidget class does include the Q_OBJECT macro. I'm thinking that it might be CMake breaking stuff or maybe it's because everything in inlined in that widget class as I orginally created it to test stuff before keeping it.

JonB

@Deneguil
Do one complete, really clean rebuild (when Q_OBJECT is involved)?

Just for the future

Oh good catch, I thought it was saying Function instead.

&AutomatLab::StateParams isn't even a (plain) function. Per the other param it's a PointerToMemberFunction, the member-ness makes it different. The 3 parameter overload would require a free function (or nowadays also a lambda, which is what you would use it for), or similar.

Christian Ehrlicher

@Deneguil said in Connection signals for dynamically created widgets:

connect(sw, &StateWidget::clicked, this, &AutomatLab::StateParams);

I doubt 'this' aka GraphicView is derived from AutomatLab ... please read the documentation on how signals and slots work and what you have to pass to the connect statement... https://doc.qt.io/qt-6/signalsandslots.html

JonB

@Christian-Ehrlicher said in Connection signals for dynamically created widgets:

I doubt 'this' aka GraphicView is derived from AutomatLab

Oh!!!

Deneguil

@Christian-Ehrlicher said in Connection signals for dynamically created widgets:

I doubt 'this' aka GraphicView is derived from AutomatLab ... please read the documentation on how signals and slots work and what you have to pass to the connect statement... https://doc.qt.io/qt-6/signalsandslots.html

The this in this case is the instance of AutomatLab in this case! I tried to use the comment to indicate it was in the constructor of the AutomatLab.cpp after forwarding it but I should've used a different code block that's my bad

@JonB said in Connection signals for dynamically created widgets:

Do one complete, really clean rebuild (when Q_OBJECT is involved)?

I deleted the build folder, restarted vscode as it's not the first time CMake got a bit confused and sometimes restarting it fixed it and did a clean rebuild and still the same error. Which is weird to me as QGraphicsEllipseItem inherits from QObject so the macro should work

JonB

@Deneguil said in Connection signals for dynamically created widgets:

as QGraphicsEllipseItem inherits from QObject

But it does not! QGraphicsItems do not inherit QObject, only QGraphicsObjects do. If StateWidget is a QGraphicsElipseItem (you never said what it is) you will have to multi-inherit to add QObject if you want to send signals from it.

Deneguil

@JonB said in Connection signals for dynamically created widgets:

Bit it does not! QGraphicsItems do not inherit QObject, only QGraphicsObjects do.

You're right! I got confused when going up the inheritance tree in the documentation, I clicked on "inherited by QGraphicsObject" at some point thinking it was "inherits" instead.

I didn't specify in a written manner that my StateWidget was a QGraphicsEllipseItem as I had added the declaration of the class in the first message. I changed the class to inherit QObject as well though and it worked!

class StateWidget : public QObject, public QGraphicsEllipseItem { ... }

The order of inheritance is important too.

The "Test n" below comes from the AutomatLab::StateParams function so the signal is properly forwarded to the main UI instance!

Thank you for your help!