From 6cad2bc8341a10eb3ebde75e987a9baac94e3032 Mon Sep 17 00:00:00 2001 From: WandererFan Date: Sat, 7 Jan 2017 11:12:17 -0500 Subject: [PATCH] Delete DPGI's when parent DPG deleted Ensure Gui/QGIVxx is removed when App/DVxxxx is deleted --- src/Mod/TechDraw/App/DrawProjGroup.cpp | 8 +- src/Mod/TechDraw/App/DrawProjGroupItem.cpp | 4 +- src/Mod/TechDraw/App/DrawViewCollection.cpp | 23 ++++ src/Mod/TechDraw/App/DrawViewCollection.h | 8 +- src/Mod/TechDraw/Gui/MDIViewPage.cpp | 102 +++++++++++++----- src/Mod/TechDraw/Gui/MDIViewPage.h | 5 + src/Mod/TechDraw/Gui/QGIView.cpp | 15 +-- src/Mod/TechDraw/Gui/QGVPage.cpp | 46 ++++++-- src/Mod/TechDraw/Gui/QGVPage.h | 6 ++ .../TechDraw/Gui/ViewProviderDrawingView.cpp | 13 --- .../TechDraw/Gui/ViewProviderDrawingView.h | 3 +- 11 files changed, 172 insertions(+), 61 deletions(-) diff --git a/src/Mod/TechDraw/App/DrawProjGroup.cpp b/src/Mod/TechDraw/App/DrawProjGroup.cpp index 926a449b8..4b00e73ce 100644 --- a/src/Mod/TechDraw/App/DrawProjGroup.cpp +++ b/src/Mod/TechDraw/App/DrawProjGroup.cpp @@ -176,7 +176,9 @@ void DrawProjGroup::onChanged(const App::Property* prop) TechDraw::DrawPage *page = getPage(); if (!isRestoring() && page) { if ( prop == &Views ) { - recompute(); + if (!isDeleting()) { + recompute(); + } } else if (prop == &Scale) { updateChildren(Scale.getValue()); //resetPositions(); @@ -808,7 +810,9 @@ TechDraw::DrawProjGroupItem* DrawProjGroup::getAnchor(void) App::DocumentObject* docObj = Anchor.getValue(); if (docObj == nullptr) { //explode! DPG w/o anchor - Base::Console().Error("Error - DPG::getAnchor - DPG has no Anchor!!!\n"); + if (!isDeleting()) { + Base::Console().Error("Error - DPG::getAnchor - DPG has no Anchor!!!\n"); + } } else { result = static_cast(docObj); } diff --git a/src/Mod/TechDraw/App/DrawProjGroupItem.cpp b/src/Mod/TechDraw/App/DrawProjGroupItem.cpp index f772c09f1..8aa9583dd 100644 --- a/src/Mod/TechDraw/App/DrawProjGroupItem.cpp +++ b/src/Mod/TechDraw/App/DrawProjGroupItem.cpp @@ -164,13 +164,15 @@ void DrawProjGroupItem::unsetupObject() { if (getGroup() != nullptr) { if (getGroup()->hasProjection(Type.getValueAsString()) ) { - if (getGroup()->getAnchor() == this) { + if ((getGroup()->getAnchor() == this) && + !getGroup()->isDeleting() ) { Base::Console().Warning("Warning - DPG (%s/%s) may be corrupt - Anchor deleted\n", getGroup()->getNameInDocument(),getGroup()->Label.getValue()); getGroup()->Anchor.setValue(nullptr); } } } + DrawViewPart::unsetupObject(); } PyObject *DrawProjGroupItem::getPyObject(void) diff --git a/src/Mod/TechDraw/App/DrawViewCollection.cpp b/src/Mod/TechDraw/App/DrawViewCollection.cpp index 47e6b7922..a0f158d2d 100644 --- a/src/Mod/TechDraw/App/DrawViewCollection.cpp +++ b/src/Mod/TechDraw/App/DrawViewCollection.cpp @@ -26,6 +26,8 @@ # include #endif +#include + #include #include @@ -42,6 +44,7 @@ PROPERTY_SOURCE(TechDraw::DrawViewCollection, TechDraw::DrawView) DrawViewCollection::DrawViewCollection() { + nowDeleting = false; static const char *group = "Drawing view"; ADD_PROPERTY_TYPE(Source ,(0), group, App::Prop_None,"Shape to view"); ADD_PROPERTY_TYPE(Views ,(0), group, App::Prop_None,"Attached Views"); @@ -165,6 +168,26 @@ void DrawViewCollection::onChanged(const App::Property* prop) TechDraw::DrawView::onChanged(prop); } +void DrawViewCollection::unsetupObject() +{ + nowDeleting = true; + + // Remove the collection's views from document + App::Document* doc = getDocument(); + std::string docName = doc->getName(); + + const std::vector currViews = Views.getValues(); + std::vector emptyViews; + std::vector::const_iterator it = currViews.begin(); + for (; it != currViews.end(); it++) { + std::string viewName = (*it)->getNameInDocument(); + Base::Interpreter().runStringArg("App.getDocument(\"%s\").removeObject(\"%s\")", + docName.c_str(), viewName.c_str()); + } + Views.setValues(emptyViews); +} + + App::DocumentObjectExecReturn *DrawViewCollection::execute(void) { if (ScaleType.isValue("Page")) { diff --git a/src/Mod/TechDraw/App/DrawViewCollection.h b/src/Mod/TechDraw/App/DrawViewCollection.h index 4b1c85d54..d0e0ea029 100644 --- a/src/Mod/TechDraw/App/DrawViewCollection.h +++ b/src/Mod/TechDraw/App/DrawViewCollection.h @@ -50,14 +50,13 @@ public: int addView(DrawView *view); int removeView(DrawView *view); void rebuildViewList(void); + bool isDeleting(void) { return nowDeleting; } int countChildren(); - /** @name methods overide Feature */ - //@{ - /// recalculate the Feature + virtual void onDocumentRestored(); virtual App::DocumentObjectExecReturn *execute(void); - //@} + virtual void unsetupObject(); /// returns the type name of the ViewProvider virtual const char* getViewProviderName(void) const { @@ -67,6 +66,7 @@ public: protected: void onChanged(const App::Property* prop); + bool nowDeleting; }; } //namespace TechDraw diff --git a/src/Mod/TechDraw/Gui/MDIViewPage.cpp b/src/Mod/TechDraw/Gui/MDIViewPage.cpp index 427bb0c85..dd6fffdcd 100644 --- a/src/Mod/TechDraw/Gui/MDIViewPage.cpp +++ b/src/Mod/TechDraw/Gui/MDIViewPage.cpp @@ -38,6 +38,9 @@ #include #include #include + #include + #include + #endif // #ifndef _PreComp_ #include @@ -146,6 +149,12 @@ MDIViewPage::MDIViewPage(ViewProviderPage *pageVp, Gui::Document* doc, QWidget* m_view->scene(), SIGNAL(selectionChanged()), this , SLOT (selectionChanged()) ); + + //get informed by App side about deleted DocumentObjects + App::Document* appDoc = m_vpPage->getDocument()->getDocument(); + auto bnd = boost::bind(&MDIViewPage::onDeleteObject, this, _1); + connectDeletedObject = appDoc->signalDeletedObject.connect(bnd); + // A fresh page is added and we iterate through its collected children and add these to Canvas View -MLP // if docobj is a featureviewcollection (ex orthogroup), add its child views. if there are ever children that have children, @@ -181,6 +190,7 @@ MDIViewPage::MDIViewPage(ViewProviderPage *pageVp, Gui::Document* doc, QWidget* MDIViewPage::~MDIViewPage() { + connectDeletedObject.disconnect(); } @@ -323,7 +333,18 @@ void MDIViewPage::removeView(QGIView *view) (void) m_view->removeView(view); } - +void MDIViewPage::onDeleteObject(const App::DocumentObject& obj) +{ + if (obj.isDerivedFrom(TechDraw::DrawView::getClassTypeId())) { + const App::DocumentObject* objPtr = &obj; + const TechDraw::DrawView* dv = static_cast(objPtr); + TechDraw::DrawPage* dvPg = dv->findParentPage(); + if (dvPg == m_vpPage->getDrawPage()) { + //this is a DV that is on our page + (void) m_view->removeView(dv); + } + } +} void MDIViewPage::updateTemplate(bool forceUpdate) { @@ -353,10 +374,19 @@ void MDIViewPage::updateTemplate(bool forceUpdate) void MDIViewPage::updateDrawing(bool forceUpdate) { - // We cannot guarantee if the number of graphical representations (QGIVxxxx) have changed so check the number (MLP) + // We cannot guarantee if the number of graphical representations (QGIVxxxx) have changed so check the number (MLP) + // WF: this should be fixed now with onDeletedObject signal from App side? + // WF: the QGVP views list may still not be 100% reliable + // TODO: build list of QGIV's from scene everytime? + //logging counters + int qgvpIn = 0; + int qgvpValid = 0; + int qgvpClean = 0; + int dpIn = 0; const std::vector &graphicsList = m_view->getViews(); + qgvpIn = graphicsList.size(); const std::vector &pageChildren = m_vpPage->getDrawPage()->Views.getValues(); - + // Count total # DocumentObjects in Page unsigned int docObjCount = 0; for(std::vector::const_iterator it = pageChildren.begin(); it != pageChildren.end(); ++it) { @@ -367,7 +397,12 @@ void MDIViewPage::updateDrawing(bool forceUpdate) } docObjCount += 1; } - + dpIn = docObjCount; + + + //TODO: should prune QGVP.views first always, then check if view in Page missing QGIVP + // this makes assumption that = numbers mean everythign is OK, but could be double failure - 1 extra QGIV, 1 DV missing graphics! + if(graphicsList.size() < docObjCount) { // there are more DocumentObjects than graphical representations (QGIVxxxx's) // Find which DocumentObjects have no graphical representation (QGIVxxxx) @@ -379,31 +414,50 @@ void MDIViewPage::updateDrawing(bool forceUpdate) attachView(*it); } } else if(graphicsList.size() > docObjCount) { - // There are more graphical representations (QGIVxxxx) than DocumentObjects - // Remove the orphans - std::vector::const_iterator itGraphics = graphicsList.begin(); + // prune any invalid entries in QGVP.views + // TODO: revisit this mess. is it still required with onDeletedItem signal implementation? std::vector newGraphicsList; - bool fnd = false; - while(itGraphics != graphicsList.end()) { - fnd = orphanExists((*itGraphics)->getViewName(), pageChildren); - if(fnd) { - newGraphicsList.push_back(*itGraphics); - } else { - if (m_view->scene() == (*itGraphics)->scene()) { - (*itGraphics)->hide(); - m_view->scene()->removeItem(*itGraphics); - } else { // this "shouldn't" happen, but it does - Base::Console().Log("ERROR - MDIViewPage::updateDrawing - %s already removed from QGraphicsScene\n", - (*itGraphics)->getViewName()); + QList items = m_view->scene()->items(); + for (auto& v: graphicsList) { //check that everything in QGVP views is valid + for (auto& i:items) { + if (v == i) { //this one is OK + newGraphicsList.push_back(v); + break; } } - itGraphics++; } - - // Update the QGVPage (QGraphicsView) list of QGIVxxxx - m_view->setViews(newGraphicsList); + qgvpValid = newGraphicsList.size(); + //newGraphicsList now only contains valid QGIV's + //now prune the ones without docObjs + std::vector cleanItems; + for (auto& i: newGraphicsList) { + std::string viewName = (i->data(1).toString()).toStdString(); + App::DocumentObject* dObj = getAppDocument()->getObject(viewName.c_str()); + if (dObj == nullptr) { + //need to remove from group/scene + QGraphicsItemGroup* grp = i->group(); + if (grp) { + grp->removeFromGroup(i); + } + if (i->parentItem()) { //not top level + i->setParentItem(0); + } + if (i->scene()) { + i->scene()->removeItem(i); + } + //should delete i too to prevent leak? might be garbage pointer, though. + //add to delete list and delete outside of loop + } else { + QGIView* v = static_cast(i); + cleanItems.push_back(v); + } + } + qgvpClean = cleanItems.size(); + m_view->setViews(cleanItems); + Base::Console().Message("Log - MDIVP::updateDrawing pruning: docObjs: %d views in: %d valid views: %d views out: %d\n", + dpIn,qgvpIn,qgvpValid, qgvpClean); } - + // Update all the QGIVxxxx const std::vector &upviews = m_view->getViews(); for(std::vector::const_iterator it = upviews.begin(); it != upviews.end(); ++it) { diff --git a/src/Mod/TechDraw/Gui/MDIViewPage.h b/src/Mod/TechDraw/Gui/MDIViewPage.h index 5126273fd..13fab0db8 100644 --- a/src/Mod/TechDraw/Gui/MDIViewPage.h +++ b/src/Mod/TechDraw/Gui/MDIViewPage.h @@ -111,6 +111,11 @@ protected: QPrinter::PaperSize getPaperSize(int w, int h) const; void setDimensionGroups(void); void showStatusMsg(const char* s1, const char* s2, const char* s3) const; + + void onDeleteObject(const App::DocumentObject& obj); + + typedef boost::BOOST_SIGNALS_NAMESPACE::connection Connection; + Connection connectDeletedObject; private: QAction *m_nativeAction; diff --git a/src/Mod/TechDraw/Gui/QGIView.cpp b/src/Mod/TechDraw/Gui/QGIView.cpp index 336c4c913..61f201f4e 100644 --- a/src/Mod/TechDraw/Gui/QGIView.cpp +++ b/src/Mod/TechDraw/Gui/QGIView.cpp @@ -189,12 +189,8 @@ void QGIView::mouseReleaseEvent(QGraphicsSceneMouseEvent * event) if (!isInnerView()) { double tempX = x(), tempY = getY(); -// getViewObject()->X.setValue(tempX); -// getViewObject()->Y.setValue(tempY); getViewObject()->setPosition(Rez::appX(tempX),Rez::appX(tempY)); } else { -// getViewObject()->X.setValue(x()); -// getViewObject()->Y.setValue(getYInClip(y())); getViewObject()->setPosition(Rez::appX(x()),Rez::appX(getYInClip(y()))); } getViewObject()->setMouseMove(false); @@ -297,11 +293,10 @@ void QGIView::setViewFeature(TechDraw::DrawView *obj) viewObj = obj; viewName = obj->getNameInDocument(); - - // Set the QGIGroup initial position based on the DrawView ( wrong. new views are always at Page center) -// float x = obj->X.getValue(); -// float y = obj->Y.getValue(); -// setPosition(x, y); + + //mark the actual QGraphicsItem so we can check what's in the scene later + setData(0,QString::fromUtf8("QGIV")); + setData(1,QString::fromUtf8(obj->getNameInDocument())); } void QGIView::toggleCache(bool state) @@ -410,7 +405,7 @@ void QGIView::paint(QPainter *painter, const QStyleOptionGraphicsItem *option, Q QStyleOptionGraphicsItem myOption(*option); myOption.state &= ~QStyle::State_Selected; - //painter->drawRect(boundingRect()); + //painter->drawRect(boundingRect()); //good for debugging QGraphicsItemGroup::paint(painter, &myOption, widget); } diff --git a/src/Mod/TechDraw/Gui/QGVPage.cpp b/src/Mod/TechDraw/Gui/QGVPage.cpp index ac41d8f43..c91617231 100644 --- a/src/Mod/TechDraw/Gui/QGVPage.cpp +++ b/src/Mod/TechDraw/Gui/QGVPage.cpp @@ -200,6 +200,7 @@ int QGVPage::addView(QGIView *view) int QGVPage::removeView(QGIView *view) { + std::vector qviews = views; std::vector newViews; @@ -217,6 +218,45 @@ int QGVPage::removeView(QGIView *view) return views.size(); } + removeViewFromScene(view); + + qviews.erase(qvDel); + views = qviews; + delete view; + + return views.size(); +} + +int QGVPage::removeView(const TechDraw::DrawView* dv) +{ + std::vector newViews; + QList items = scene()->items(); + QString qsName = QString::fromUtf8(dv->getNameInDocument()); + bool found = false; + QGIView* ourItem = nullptr; + for (auto& i:items) { + if (qsName == i->data(1).toString()) { //is there really a QGIV for this DV in scene? + found = true; + ourItem = static_cast(i); + break; + } + } + if (found) { + for (auto&v :views) { + if (ourItem != v) { + newViews.push_back(v); + } + } + removeViewFromScene(ourItem); + delete ourItem; + views = newViews; + } + + return views.size(); +} + +void QGVPage::removeViewFromScene(QGIView *view) +{ QGraphicsItemGroup* grp = view->group(); if (grp) { grp->removeFromGroup(view); @@ -229,12 +269,6 @@ int QGVPage::removeView(QGIView *view) if (view->scene()) { view->scene()->removeItem(view); } - - qviews.erase(qvDel); - views = qviews; - delete view; - - return views.size(); } diff --git a/src/Mod/TechDraw/Gui/QGVPage.h b/src/Mod/TechDraw/Gui/QGVPage.h index 8ca6a9e59..7223650a2 100644 --- a/src/Mod/TechDraw/Gui/QGVPage.h +++ b/src/Mod/TechDraw/Gui/QGVPage.h @@ -27,6 +27,7 @@ #include namespace TechDraw { +class DrawView; class DrawViewPart; class DrawProjGroup; class DrawViewDimension; @@ -78,8 +79,11 @@ public: void addDimToParent(QGIViewDimension* dim, QGIView* parent); const std::vector & getViews() const { return views; } + int addView(QGIView * view); int removeView(QGIView *view); + int removeView(const TechDraw::DrawView* dv); + void setViews(const std::vector &view) {views = view; } void setPageTemplate(TechDraw::DrawTemplate *pageTemplate); @@ -107,6 +111,8 @@ protected: static QColor SelectColor; static QColor PreselectColor; QColor getBackgroundColor(); + + void removeViewFromScene(QGIView *view); QGITemplate *pageTemplate; std::vector views; diff --git a/src/Mod/TechDraw/Gui/ViewProviderDrawingView.cpp b/src/Mod/TechDraw/Gui/ViewProviderDrawingView.cpp index 2ccbda836..0a118fc9b 100644 --- a/src/Mod/TechDraw/Gui/ViewProviderDrawingView.cpp +++ b/src/Mod/TechDraw/Gui/ViewProviderDrawingView.cpp @@ -188,7 +188,6 @@ void ViewProviderDrawingView::updateData(const App::Property* prop) qgiv->updateView(true); } } - Gui::ViewProviderDocumentObject::updateData(prop); } @@ -202,18 +201,6 @@ void ViewProviderDrawingView::unsetEdit(int ModNum) } } -bool ViewProviderDrawingView::onDelete(const std::vector &items) -{ - QGIView* qv = getQView(); - if (qv != nullptr) { - MDIViewPage* mdi = getMDIViewPage(); - mdi->removeView(qv); - } - - return ViewProviderDocumentObject::onDelete(items); -} - - MDIViewPage* ViewProviderDrawingView::getMDIViewPage() const { MDIViewPage* result = nullptr; diff --git a/src/Mod/TechDraw/Gui/ViewProviderDrawingView.h b/src/Mod/TechDraw/Gui/ViewProviderDrawingView.h index a5d2bfe43..acf90a094 100644 --- a/src/Mod/TechDraw/Gui/ViewProviderDrawingView.h +++ b/src/Mod/TechDraw/Gui/ViewProviderDrawingView.h @@ -60,7 +60,6 @@ public: virtual void onChanged(const App::Property *prop); virtual void updateData(const App::Property*); virtual void unsetEdit(int ModNum); - virtual bool onDelete(const std::vector &items); QGIView* getQView(void); MDIViewPage* getMDIViewPage() const; @@ -70,10 +69,12 @@ public: virtual void startRestoring(); virtual void finishRestoring(); //@} + virtual TechDraw::DrawView* getViewObject() const; private: bool m_docReady; //sb MDI + QGraphicsScene ready + }; } // namespace TechDrawGui