# HG changeset patch # User Santeri Piippo # Date 1400192544 -10800 # Node ID 8bb3bed445704a725d8c5eeeb8a5fa31382f7e5c # Parent 29e6c5552388543193e254281d5fd9ba6889700d - improved shared pointer behavior, still not there yet diff -r 29e6c5552388 -r 8bb3bed44570 src/actionsEdit.cc --- a/src/actionsEdit.cc Fri May 09 12:06:56 2014 +0300 +++ b/src/actionsEdit.cc Fri May 16 01:22:24 2014 +0300 @@ -177,7 +177,6 @@ // after the first one. getCurrentDocument()->setObject (index, triangles[0]); getCurrentDocument()->insertObj (index + 1, triangles[1]); - obj->destroy(); num++; } diff -r 29e6c5552388 -r 8bb3bed44570 src/addObjectDialog.cc --- a/src/addObjectDialog.cc Fri May 09 12:06:56 2014 +0300 +++ b/src/addObjectDialog.cc Fri May 16 01:22:24 2014 +0300 @@ -265,9 +265,7 @@ layout->addWidget (bbx_buttons, 5, 0, 1, 4); setLayout (layout); setWindowTitle (format (tr ("Edit %1"), typeName)); - setWindowIcon (icon); - defaults->destroy(); } // ============================================================================= diff -r 29e6c5552388 -r 8bb3bed44570 src/ldDocument.cc --- a/src/ldDocument.cc Fri May 09 12:06:56 2014 +0300 +++ b/src/ldDocument.cc Fri May 16 01:22:24 2014 +0300 @@ -379,7 +379,6 @@ line.chop (1); LDObjectPtr obj = parseLine (line); - fprint (stderr, "got obj: %1\n", obj.data()); // Check for parse errors and warn about tthem if (obj->type() == LDObject::EError) diff -r 29e6c5552388 -r 8bb3bed44570 src/ldObject.cc --- a/src/ldObject.cc Fri May 09 12:06:56 2014 +0300 +++ b/src/ldObject.cc Fri May 16 01:22:24 2014 +0300 @@ -32,7 +32,10 @@ CFGENTRY (Int, defaultLicense, 0); // List of all LDObjects -static LDObjectWeakList g_LDObjects; +static QMap g_allObjects; + +// Temporary resource +LDObjectPtr g_temporaryLDObject; // ============================================================================= // LDObject constructors @@ -40,13 +43,20 @@ LDObject::LDObject() : m_isHidden (false), m_isSelected (false), + m_isDestructed (false), m_document (null), qObjListEntry (null) { + LDObjectPtr selfptr (this, [](LDObject* obj){ obj->finalDelete(); }); memset (m_coords, 0, sizeof m_coords); + m_self = selfptr; chooseID(); - g_LDObjects << LDObjectWeakPtr (thisptr()); + g_allObjects[id()] = self(); setRandomColor (QColor::fromHsv (rand() % 360, rand() % 256, rand() % 96 + 128)); + + // This prevents the object from being prematurely deleted just because + // selfptr falls out of scope before it's caught by spawn() + g_temporaryLDObject = selfptr; } // ============================================================================= @@ -55,13 +65,18 @@ { int32 id = 1; // 0 shalt be null - for (LDObjectWeakPtr obj : g_LDObjects) + for (auto it = g_allObjects.begin(); it != g_allObjects.end(); ++it) { - LDObjectPtr obj2 = obj.toStrongRef(); - assert (obj2 != this); + LDObjectPtr obj = it->toStrongRef(); + + if (not obj) + fprint (stdout, "obj #%1 wasn't removed properly\n", it.key()); - if (obj2->id() >= id) - id = obj2->id() + 1; + assert (obj != this); + assert (obj != null); + + if (obj->id() >= id) + id = obj->id() + 1; } setID (id); @@ -226,7 +241,7 @@ void LDObject::swap (LDObjectPtr other) { assert (document() == other->document()); - document()->swapObjects (thisptr(), other); + document()->swapObjects (self(), other); } // ============================================================================= @@ -263,10 +278,6 @@ // ============================================================================= // -LDSubfile::~LDSubfile() {} - -// ============================================================================= -// void LDObject::destroy() { // If this object was selected, unselect it now @@ -275,13 +286,24 @@ // If this object was associated to a file, remove it off it now if (document()) - document()->forgetObject (thisptr()); + document()->forgetObject (self()); // Delete the GL lists - g_win->R()->forgetObject (thisptr()); + g_win->R()->forgetObject (self()); // Remove this object from the list of LDObjects - g_LDObjects.removeOne (LDObjectWeakPtr (thisptr())); + g_allObjects.erase (g_allObjects.find (id())); + setDestructed (true); +} + +// +// Deletes the object. Only the shared pointer is to call this! +// +void LDObject::finalDelete() +{ + if (not isDestructed()) + destroy(); + delete this; } @@ -334,7 +356,7 @@ for (LDObjectPtr obj : objs) { // Set the parent now so we know what inlined the object. - obj->setParent (LDObjectWeakPtr (thisptr())); + obj->setParent (self()); transformObject (obj, transform(), position(), color()); } @@ -440,10 +462,7 @@ // String LDObject::typeName (LDObject::Type type) { - LDObjectPtr obj = LDObject::getDefault (type); - String name = obj->typeName(); - obj->destroy(); - return name; + return LDObject::getDefault (type)->typeName(); } // ============================================================================= @@ -486,9 +505,9 @@ LDObjectPtr LDObject::topLevelParent() { if (parent() == null) - return thisptr(); + return self(); - LDObjectWeakPtr it (thisptr()); + LDObjectWeakPtr it (self()); while (it.toStrongRef()->parent() != null) it = it.toStrongRef()->parent(); @@ -528,13 +547,13 @@ { if (hasMatrix()) { - LDMatrixObjectPtr mo = thisptr().dynamicCast(); + LDMatrixObjectPtr mo = self().toStrongRef().dynamicCast(); mo->setPosition (mo->position() + vect); } elif (type() == LDObject::EVertex) { // ugh - thisptr().staticCast()->pos += vect; + self().toStrongRef().staticCast()->pos += vect; } else { @@ -664,13 +683,10 @@ // LDObjectPtr LDObject::fromID (int id) { - for (LDObjectWeakPtr obj : g_LDObjects) - { - LDObjectPtr obj2 = obj.toStrongRef(); + auto it = g_allObjects.find (id); - if (obj2->id() == id) - return obj; - } + if (it != g_allObjects.end()) + return *it; return LDObjectPtr(); } @@ -719,7 +735,7 @@ // void LDObject::setColor (const int& val) { - changeProperty (thisptr(), &m_color, val); + changeProperty (self(), &m_color, val); } // ============================================================================= @@ -736,33 +752,37 @@ if (document() != null) document()->vertexChanged (*m_coords[i], vert); - changeProperty (thisptr(), &m_coords[i], LDSharedVertex::getSharedVertex (vert)); + changeProperty (self(), &m_coords[i], LDSharedVertex::getSharedVertex (vert)); } // ============================================================================= // void LDMatrixObject::setPosition (const Vertex& a) { - if (linkPointer()->document() != null) - linkPointer()->document()->removeKnownVerticesOf (linkPointer()); + LDObjectPtr ref = linkPointer().toStrongRef(); + + if (ref->document() != null) + ref->document()->removeKnownVerticesOf (ref); - changeProperty (linkPointer(), &m_position, LDSharedVertex::getSharedVertex (a)); + changeProperty (ref, &m_position, LDSharedVertex::getSharedVertex (a)); - if (linkPointer()->document() != null) - linkPointer()->document()->addKnownVerticesOf (linkPointer()); + if (ref->document() != null) + ref->document()->addKnownVerticesOf (ref); } // ============================================================================= // void LDMatrixObject::setTransform (const Matrix& val) { - if (linkPointer()->document() != null) - linkPointer()->document()->removeKnownVerticesOf (linkPointer()); + LDObjectPtr ref = linkPointer().toStrongRef(); + + if (ref->document() != null) + ref->document()->removeKnownVerticesOf (ref); - changeProperty (linkPointer(), &m_transform, val); + changeProperty (ref, &m_transform, val); - if (linkPointer()->document() != null) - linkPointer()->document()->addKnownVerticesOf (linkPointer()); + if (ref->document() != null) + ref->document()->addKnownVerticesOf (ref); } // ============================================================================= @@ -808,7 +828,7 @@ void LDObject::select() { assert (document() != null); - document()->addToSelection (thisptr()); + document()->addToSelection (self()); } // ============================================================================= @@ -816,7 +836,7 @@ void LDObject::deselect() { assert (document() != null); - document()->removeFromSelection (thisptr()); + document()->removeFromSelection (self()); } // ============================================================================= @@ -852,7 +872,7 @@ void LDSubfile::setFileInfo (const LDDocumentPointer& a) { if (document() != null) - document()->removeKnownVerticesOf (thisptr()); + document()->removeKnownVerticesOf (self()); m_fileInfo = a; @@ -867,5 +887,5 @@ } if (document() != null) - document()->addKnownVerticesOf (thisptr()); + document()->addKnownVerticesOf (self()); }; diff -r 29e6c5552388 -r 8bb3bed44570 src/ldObject.h --- a/src/ldObject.h Fri May 09 12:06:56 2014 +0300 +++ b/src/ldObject.h Fri May 16 01:22:24 2014 +0300 @@ -31,6 +31,7 @@ } \ virtual String asText() const override; \ virtual void invert() override; \ + \ protected: \ friend class QSharedPointer::ExternalRefCount; \ @@ -65,11 +66,13 @@ { PROPERTY (public, bool, isHidden, setHidden, STOCK_WRITE) PROPERTY (public, bool, isSelected, setSelected, STOCK_WRITE) + PROPERTY (public, bool, isDestructed, setDestructed, STOCK_WRITE) PROPERTY (public, LDObjectWeakPtr, parent, setParent, STOCK_WRITE) PROPERTY (public, LDDocument*, document, setDocument, STOCK_WRITE) PROPERTY (private, int, id, setID, STOCK_WRITE) PROPERTY (public, int, color, setColor, CUSTOM_WRITE) PROPERTY (private, QColor, randomColor, setRandomColor, STOCK_WRITE) + PROPERTY (private, LDObjectWeakPtr, self, setSelf, STOCK_WRITE) public: // Object type codes. @@ -177,8 +180,12 @@ // TODO: make this private! QListWidgetItem* qObjListEntry; + // This is public because I cannot protect it as the lambda deletor would + // have to be the friend. Do not call this! Ever! + void finalDelete(); + protected: - // LDObjects are to be deleted with the deleteSelf() method, not with + // LDObjects are to be deleted with the finalDelete() method, not with // operator delete. This is because it seems virtual functions cannot // be properly called from the destructor, thus a normal method must // be used instead. The destructor also doesn't seem to be able to @@ -191,26 +198,23 @@ // pointer's base class still calls operator delete directly in one of // its methods. The method should never be called but we need to declare // the class making this delete call a friend anyway. + friend class QSharedPointer; friend class QSharedPointer::ExternalRefCount; - inline LDObjectPtr thisptr() - { - return LDObjectPtr (this); - } - private: LDSharedVertex* m_coords[4]; }; // // Makes a new LDObject. This makes the shared pointer always use the custom -// deleter so that all deletions go through destroy(); +// deleter so that all deletions go through finalDelete(); // template inline QSharedPointer spawn (Args... args) { static_assert (std::is_base_of::value, "spawn may only be used with LDObject-derivatives"); - return QSharedPointer (new T (args...), [](T* obj){ obj->destroy(); }); + LDObject* obj = new T (args...); + return obj->self().toStrongRef().staticCast(); } NUMERIC_ENUM_OPERATORS (LDObject::Type) @@ -271,8 +275,8 @@ // class LDMatrixObject { - PROPERTY (public, LDObjectPtr, linkPointer, setLinkPointer, STOCK_WRITE) - PROPERTY (public, Matrix, transform, setTransform, CUSTOM_WRITE) + PROPERTY (public, LDObjectWeakPtr, linkPointer, setLinkPointer, STOCK_WRITE) + PROPERTY (public, Matrix, transform, setTransform, CUSTOM_WRITE) public: LDMatrixObject() : @@ -446,18 +450,15 @@ Q_DECLARE_FLAGS (InlineFlags, InlineFlag) - LDSubfile() + LDSubfile() : + LDObject() { - setLinkPointer (QSharedPointer (this)); + setLinkPointer (self()); } - // Inlines this subfile. Note that return type is an array of heap-allocated - // LDObject copies, they must be deleted manually. + // Inlines this subfile. LDObjectList inlineContents (bool deep, bool render); QList inlinePolygons(); - - protected: - ~LDSubfile(); }; Q_DECLARE_OPERATORS_FOR_FLAGS (LDSubfile::InlineFlags) diff -r 29e6c5552388 -r 8bb3bed44570 src/main.cc --- a/src/main.cc Fri May 09 12:06:56 2014 +0300 +++ b/src/main.cc Fri May 16 01:22:24 2014 +0300 @@ -43,12 +43,6 @@ CFGENTRY (Bool, firstStart, true); -static void deleteline (LDLine* obj) -{ - fprintf (stderr, "delete %p\n", obj); - delete obj; -} - // ============================================================================= // int main (int argc, char* argv[]) @@ -74,15 +68,7 @@ initColors(); MainWindow* win = new MainWindow; newFile(); - //win->show(); - - QSharedPointer obj (new LDLine, &deleteline); - fprint (stderr, "%1: %2\n", obj.data(), obj->type()); - //class A { public: virtual ~A(){} virtual int foo() = 0; void destroy() { delete this; }}; - //class B : public A { public: virtual ~B(){} virtual int foo() override { return 5; }}; - //QSharedPointer a (new B, [](A* p) { p->destroy(); }); - //fprintf (stderr, "%p (%lu bytes)\n", a.data(), sizeof *a); - exit (0); + win->show(); // If this is the first start, get the user to configuration. Especially point // them to the profile tab, it's the most important form to fill in.