Delete DPGI's when parent DPG deleted

Ensure Gui/QGIVxx is removed when App/DVxxxx is deleted

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<DrawProjGroupItem*>(docObj);
     }

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)

src/Mod/TechDraw/App/DrawViewCollection.cpp

@@ -26,6 +26,8 @@
 # include <sstream>
 #endif
 
+#include <App/Document.h>
+
 #include <Base/Console.h>
 #include <Base/Exception.h>
 
@@ -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<App::DocumentObject*> currViews = Views.getValues();
+    std::vector<App::DocumentObject*> emptyViews;
+    std::vector<App::DocumentObject*>::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")) {

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

src/Mod/TechDraw/Gui/MDIViewPage.cpp

@@ -38,6 +38,9 @@
     #include <QPrinter>
     #include <QPrintDialog>
     #include <QPrintPreviewDialog>
+    #include <boost/signal.hpp>
+    #include <boost/bind.hpp>
+
 #endif  // #ifndef _PreComp_
 
 #include <math.h>
@@ -147,6 +150,12 @@ MDIViewPage::MDIViewPage(ViewProviderPage *pageVp, Gui::Document* doc, QWidget*
         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,
      // we'll have to make this recursive. -WF
@@ -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<const TechDraw::DrawView*>(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,8 +374,17 @@ 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<QGIView *> &graphicsList = m_view->getViews();
+    qgvpIn = graphicsList.size();
     const std::vector<App::DocumentObject*> &pageChildren  = m_vpPage->getDrawPage()->Views.getValues();
    
     // Count total # DocumentObjects in Page
@@ -367,6 +397,11 @@ 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)
@@ -379,29 +414,48 @@ void MDIViewPage::updateDrawing(bool forceUpdate)
             attachView(*it);
         }
     } else if(graphicsList.size() > docObjCount) {
-        // There are more graphical representations (QGIVxxxx) than DocumentObjects
+        // prune any invalid entries in QGVP.views
-        // Remove the orphans
+        // TODO: revisit this mess.  is it still required with onDeletedItem signal implementation?
-        std::vector<QGIView *>::const_iterator itGraphics = graphicsList.begin();
         std::vector<QGIView *> newGraphicsList;
-        bool fnd = false;
+        QList<QGraphicsItem *> items = m_view->scene()->items();
-        while(itGraphics != graphicsList.end()) {
+        for (auto& v: graphicsList) {              //check that everything in QGVP views is valid
-            fnd = orphanExists((*itGraphics)->getViewName(), pageChildren);
+            for (auto& i:items) {
-            if(fnd) {
+                if (v == i) {                      //this one is OK
-                newGraphicsList.push_back(*itGraphics);
+                    newGraphicsList.push_back(v);
-            } else {
+                    break;
-                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());
                 }
             }
-            itGraphics++;
         }
-
+        qgvpValid = newGraphicsList.size();
-        // Update the QGVPage (QGraphicsView) list of QGIVxxxx
+        //newGraphicsList now only contains valid QGIV's
-        m_view->setViews(newGraphicsList);
+        //now prune the ones without docObjs
+        std::vector<QGIView *> 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<QGIView*>(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

src/Mod/TechDraw/Gui/MDIViewPage.h

@@ -112,6 +112,11 @@ protected:
     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;
     QAction *m_glAction;

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);
@@ -298,10 +294,9 @@ 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)
+    //mark the actual QGraphicsItem so we can check what's in the scene later
-//    float x = obj->X.getValue();
+    setData(0,QString::fromUtf8("QGIV"));
-//    float y = obj->Y.getValue();
+    setData(1,QString::fromUtf8(obj->getNameInDocument()));
-//    setPosition(x, y);
 }
 
 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);
 }

src/Mod/TechDraw/Gui/QGVPage.cpp

@@ -200,6 +200,7 @@ int QGVPage::addView(QGIView *view)
 
 int QGVPage::removeView(QGIView *view)
 {
+
     std::vector<QGIView *> qviews = views;
     std::vector<QGIView *> 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<QGIView *> newViews;
+    QList<QGraphicsItem *> 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<QGIView*>(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();
 }
 
 

src/Mod/TechDraw/Gui/QGVPage.h

@@ -27,6 +27,7 @@
 #include <QGraphicsScene>
 
 namespace TechDraw {
+class DrawView;
 class DrawViewPart;
 class DrawProjGroup;
 class DrawViewDimension;
@@ -78,8 +79,11 @@ public:
 
     void addDimToParent(QGIViewDimension* dim, QGIView* parent);
     const std::vector<QGIView *> & getViews() const { return views; }
+
     int addView(QGIView * view);
     int removeView(QGIView *view);
+    int removeView(const TechDraw::DrawView* dv);
+
     void setViews(const std::vector<QGIView *> &view) {views = view; }
     void setPageTemplate(TechDraw::DrawTemplate *pageTemplate);
 
@@ -108,6 +112,8 @@ protected:
     static QColor PreselectColor;
     QColor getBackgroundColor();
     
+    void removeViewFromScene(QGIView *view);
+
     QGITemplate *pageTemplate;
     std::vector<QGIView *> views;
 

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<std::string> &items)
-{
-    QGIView* qv = getQView();
-    if (qv != nullptr) {
-        MDIViewPage* mdi = getMDIViewPage();
-        mdi->removeView(qv);
-    }
-
-    return ViewProviderDocumentObject::onDelete(items);
-}
-
-
 MDIViewPage* ViewProviderDrawingView::getMDIViewPage() const
 {
     MDIViewPage* result = nullptr;

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<std::string> &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