diff --git a/HACKING b/HACKING --- a/HACKING +++ b/HACKING @@ -15,6 +15,11 @@ easy to grasp as possible. Since people need to know Qt in the first place, keep to Qt. Discuss deviations on #krita +qobject_cast vs dynamic_cast + + When working with QObject-based classes, use qobject_cast instead of standard + dynamic_cast for conversions. It's simply faster. + C++11 and C++14 Yes, but. Avoid lambdas. Avoid the new sig/slot connection syntax _unless_ @@ -122,5 +127,142 @@ Use the standard !, !=, ==, && etc style, not the "not", "and" etc. style. Keep krita code using one, easily recognizable, C++ style. - Boudewijn Rempt + +Technical details: + +Raw pointer vs Scoped pointer vs Shared Pointer + + Prefer (local) variables on the stack (cheapest allocation wise, cannot leak, cannot be + null). If that does not work, because polymorphism is needed (the variable can be one of + several types in a type hierarchy) use QScopedPointer. This expresses clear ownership and + cannot have problems with memory management. + + When an object is owned by several places, use QSharedPointer. Objects that own the shared + pointer, have it as member. For objects that don't own an object, but refer to it, use + QWeakPointer. When then using the object, it must be checked for null. + + QWeakPointer myWeakReference = shared.toWeakRef(); + ... + // usage: + QSharedPointer foo = myWeakReference.toStrongRef(); + if (foo) { + foo->doStuff(); + } + +QSharedPointer vs KisSharedPtr + + Use QSharedPointer for any new code if possible. Quite a lot of old code uses KisSharedPtr, + a home-grown implementation of something like QSharedDataPointer. Just let this code live. + + When assigning a raw pointer to *already existing* shared pointer from a raw pointer, use + toQShared(new MyClass()) instead of specifying the the full pointer name. E.g. + + 1) On return from a function + KisPSDLayerStyleSP KisPSDLayerStyle::clone() const + { + // return type is deduced automatically + return toQShared(new KisPSDLayerStyle(*this)); + } + + 2) On passing a parameter to a function accepting shared pointer + KisViewManagerPrivate(KisViewManager *_q, QWidget *_q_parent) + { + ... + canvasResourceManager.addDerivedResourceConverter( + toQShared(new KisCompositeOpResourceConverter)); + ... + } + + 3) *But* don't add extra code when you can use an explicit constructor: + QSharePointer myFooPointer(new Foo()); // no toQShared() needed! + +KisSharedPtr and KisWeakSharedPtr + + 1) When assigning a null pointer to *already existing* shared pointer, just pass 'nullptr' + instead of specifying full pointer type name. E.g. + + 1) On return from a function: + KisNodeSP KisNode::firstChild() const + { + QReadLocker l(&m_d->nodeSubgraphLock); + return !m_d->nodes.isEmpty() ? m_d->nodes.first() : nullptr; + } + + 2) On defining a default value for a function parameter + KisPaintDevice(KisNodeWSP parent, + const KoColorSpace * colorSpace, + KisDefaultBoundsBaseSP defaultBounds = nullptr, + const QString& name = QString()); + + 2) When creating a shared pointer from a raw pointer, just use toKisShared(new MyClass()) + instead of specifying full pointer type name. + + 3) In case of a danger of cyclic or dangling pointer links, use KisWeakSharedPtr. The best + examples are nodes-to-node (parent, clones) connctions and gui-to-nodes (docker to current + node or image) connections. + + 3.1) API rule: "Object stores a weak pointer, API returns a strong pointer" + + Example: + + class MyNode { + public: + // return a strong pointer to the outer world + MyNodeSP parent() const { + return m_parent.toStrongRef(); + } + + private: + // store a weak pointer + MyNodeWSP m_parent; + }; + + 3.2) Scope rule: "Never call any function accessing weak pointers from constructor and destructor" + + Consider this (realworld) example, This code will crash because nextSibling() uses + parent() link for resolving siblings. + + class KisNode { + public: + ~KisNode() { + } + + KisNodeSP parent() const { + return m_parent.toStrongRef(); + } + + KisNodeSP firstChild() const { + return !m_children.isEMpty() ? m_children.first() : nullptr; + } + + KisNodeSP nextSibling() const { + KisNodeSP parent = this->parent(); + const int index = parent->indexOf(this); + return index >= 0 ? parent->at(index + 1) : nullptr; + } + + KisNodeSP at(int i) const { + return i < m_children.size() ? m_children[i] : nullptr; + } + + int indexOf(KisNodeSP node) const { + return m_children.indexOf(node); + } + + private: + KisNodeWSP m_parent; + QList m_childred; + }; + + class KisWeirdLayer : public KisNode { + ~KisWeirdLayer() { + KisNodeSP child = firstChild(); + while (child) { + child->doSomething(); + + // CRASH: here we enter infinite destruction loop! + child = child->nextSibling(); + } + } + };