Yes, I know. The last post on the assistants is rather boring. And yet these days I have been
working on the snapshot docker, though
it still seems a little (just a little, you see) unfinished as Dmitry is said to experience
a relatively high delay when switching between snapshots. However this is not what I can reproduce
on my older laptop, so I am really waiting for his test results in order to further investigate
the problem.
But there is something interesting happening just when I am randomly testing things. From
Krita's debug output, I saw QObject::connect() complaining about the arguments I passed,
saying it is expecting parenthesis. "Okay," I thought, "then there have to be something wrong
with the code I wrote."
And that was quite confusing. I remember having used member function pointers in those places,
got a compile-time error since KisSignalAutoConnectionsStore did not support the new syntax,
then switched back to the SINGAL() and SLOT() macros. KisSignalAutoConnectionsStore is
a helper class to quickly (dis)connect a group of connections. One can use the addConnection()
method to add a connection, and use clear() to remove all connections made before.
Well, everything good, apart from the fact that I missed the parenthesis, which I did not
discover until I looked into the debug output. So I asked Dmitry why not add the new syntax
to KisSignalAutoConnectionsStore, and he said we should.
What is good about the new syntax is compile-time checking. We probably do not want our connections
to fail to be made only when you run the program, just because there is a typo in the signature.
That is definitely tiring and hard to catch (hmm, I did not notice the problem until today I
randomly glanced at the command line; it might be worse if I shipped the snapshot docker together
with those careless bugs).
The modification to the code seems straightforward. All what happens is in the KisSignalAutoConnection
class. In its constructor, the connection is made using QObject::connect(); in its destructor,
the connection is removed by passing the same sets of arguments to QObject::disconnect() currently
in master. The signature is just KisSignalAutoConnection(const QObject *, const char *, const QObject *, const char *),
as SIGNAL() and SLOT() macros are but to append their arguments to the string "1" and "2" respectively.
So the problem we have is we do not want the arguments that specify the signals and/or slots
to be just strings. We want them to be pointers to member functions, or maybe lambdas.
According to QObject document, the signature for new-style connect() is:
QMetaObject::Connection QObject::connect(const QObject *sender, PointerToMemberFunction signal, const QObject *context, Functor functor, Qt::ConnectionType type = Qt::AutoConnection)
Okay, so we know that sender and receiver should be pointers to QObjects, and
either the type of signal or functor we do not know.
Now let's make our KisSignalAutoConnection constructor a template function:
template<class Signal, class Method>
inline KisSignalAutoConnection(const QObject *sender, Signal signal,
const QObject *receiver, Method method
Qt::ConnectionType type = Qt::AutoConnection);
But when these parameters are passed to QObject::connect(), we get a compile-time error, saying
there is no matching overload for connect().
Why?
The answer is the Qt documentation is simplifying, if not hiding, the truth. The real definition
for connect() is found in Line 227 of qobject.h:
template <typename Func1, typename Func2>
static inline QMetaObject::Connection connect(const typename QtPrivate::FunctionPointer<Func1>::Object *sender, Func1 signal,
const typename QtPrivate::FunctionPointer<Func2>::Object *receiver, Func2 slot,
Qt::ConnectionType type = Qt::AutoConnection)
And tracking down the definition of QtPrivate::FunctionPointer, we get it in qobjectdefs_impl.h:
template<class Obj, typename Ret, typename... Args> struct FunctionPointer<Ret (Obj::*) (Args...)>
{
typedef Obj Object;
...
};
And seeing what we have passed to KisSignalAutoConnection (in the test code):
KisSignalAutoConnectionsStore conn;
conn.addConnection(test1.data(), &TestClass::sigTest1, test2.data(), &TestClass::slotTest1);
We can see that Func1 is a member function of TestClass, so QtPrivate::FunctionPointer<Func1>::Object
is just TestClass. But the constructor of KisSignalAutoConnection receives a const QObject *.
The problem here is that connect() is expecting a const TestClass *, but we give them a const QObject *.
A base class pointer cannot be implicitly converted to a derived class pointer, so we have that error.
The resolution seems pretty simple, as we only need to include the types of sender and receiver
into the template, and pass everything as-is to QObject::connect():
template<class Sender, class Signal, class Receiver, class Method>
inline KisSignalAutoConnection(Sender sender, Signal signal, Receiver receiver, Method method,
Qt::ConnectionType type = Qt::AutoConnection);
Sounds viable. But how can we store the four parameters? It might be intuitive to make another base class,
say, KisSignalAutoConnectionBase(), and make KisSignalAutoConnection a template class, so we can
store sender, receiver, etc.
But wait, isn't this just too complex? First of all, we do not have any overridden functions
except for the destructor. What is more, we do not seem to have any valuable things in that base
class -- it would be an empty class. The use of inheritance here is ugly and useless.
And, we do not need to store the four parameters at all. QObject::connect() returns a
QMetaObject::Connection, which can be used later to disconnect() it. So instead of
the parameters passed to connect(), we just store the Connection object. And that is not
part of the template:
public:
template<class Sender, class Signal, class Receiver, class Method>
inline KisSignalAutoConnection(Sender sender, Signal signal, Receiver receiver, Method method,
Qt::ConnectionType type = Qt::AutoConnection)
: m_connection(QObject::connect(sender, signal, receiver, method, type))
{
}
inline ~KisSignalAutoConnection()
{
QObject::disconnect(m_connection);
}
private:
QMetaObject::Connection m_connection;
And with the test code mentioned above,
we do make sure that the new implementation works well with both syntaxes.
So, great, krita developers, we can use the new syntax for auto connections as well.
PS: There will soon be another post on my work of the snapshot docker -- it's almost finished!