small coin error

Discussion about the development of the Assembly workbench.
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
Post Reply
User avatar
tanderson69
Veteran
Posts: 1626
Joined: Thu Feb 18, 2010 1:07 am

small coin error

Post by tanderson69 »

with the current assembly: upon adding a part to the current assembly, these errors appear in the terminal.

Code: Select all

Coin error in SoGroup::removeChild(): tried to remove non-existent child 0x3194ff0 (Separator)
Coin error in SoGroup::removeChild(): tried to remove non-existent child 0x31a1fc0 (Separator)
Coin error in SoGroup::removeChild(): tried to remove non-existent child 0x31c3120 (Separator)
Traced it around and made this change and it cleared up the errors:

Code: Select all

diff --git a/src/Gui/Document.cpp b/src/Gui/Document.cpp
index e89cc6f..f01571e 100644
--- a/src/Gui/Document.cpp
+++ b/src/Gui/Document.cpp
@@ -478,7 +478,8 @@ void Document::slotChangedObject(const App::DocumentObject& Obj, const App::Prop
                             if (activeView && viewProvider) {
                                 if (d->_pcInEdit == ChildViewProvider)
                                     resetEdit();
-                                activeView->getViewer()->removeViewProvider(ChildViewProvider);
+                               if (activeView->getViewer()->hasViewProvider(ChildViewProvider))
+                                 activeView->getViewer()->removeViewProvider(ChildViewProvider);
                             }
                         }
                     }
Not sure if that is the right place to address the problem, but it should illustrate where the coin error is generated.
ickby
Veteran
Posts: 3116
Joined: Wed Oct 05, 2011 7:36 am

Re: small coin error

Post by ickby »

Seems reasonable to check if the view provider exists before removing it :) I'm adding it to my branch
User avatar
tanderson69
Veteran
Posts: 1626
Joined: Thu Feb 18, 2010 1:07 am

Re: small coin error

Post by tanderson69 »

I had it in my head there was nothing new in that function, so the problem stemmed from somewhere else.

after a "git blame", I see my assumption was wrong! First time I have used this command. 8-)

Code: Select all

git blame -L 444,496  src/Gui/Document.cpp
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 444) void Document::slotChangedObject(const App::DocumentObject& Obj, const App::Property& Prop)
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 445) {
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 446)     //Base::Console().Log("Document::slotChangedObject() called\n");
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 447)     ViewProvider* viewProvider = getViewProvider(&Obj);
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 448)     if (viewProvider) {
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 449)         try {
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 450)             viewProvider->update(&Prop);
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 451)         } catch(const Base::MemoryException& e) {
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 452)             Base::Console().Error("Memory exception in '%s' thrown: %s\n",Obj.getNameInDocument(),e.what());
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 453)         } catch(Base::Exception &e){
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 454)             e.ReportException();
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 455)         } catch (...) {
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 444) void Document::slotChangedObject(const App::DocumentObject& Obj, const App::Property& Prop)
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 445) {
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 446)     //Base::Console().Log("Document::slotChangedObject() called\n");
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 447)     ViewProvider* viewProvider = getViewProvider(&Obj);
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 448)     if (viewProvider) {
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 449)         try {
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 450)             viewProvider->update(&Prop);
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 451)         } catch(const Base::MemoryException& e) {
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 452)             Base::Console().Error("Memory exception in '%s' thrown: %s\n",Obj.getNameInDocument(),e.what());
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 453)         } catch(Base::Exception &e){
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 454)             e.ReportException();
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 455)         } catch (...) {
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 444) void Document::slotChangedObject(const App::DocumentObject& Obj, const App::Property& Prop)
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 445) {
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 446)     //Base::Console().Log("Document::slotChangedObject() called\n");
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 447)     ViewProvider* viewProvider = getViewProvider(&Obj);
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 448)     if (viewProvider) {
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 449)         try {
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 450)             viewProvider->update(&Prop);
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 451)         } catch(const Base::MemoryException& e) {
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 452)             Base::Console().Error("Memory exception in '%s' thrown: %s\n",Obj.getNameInDocument(),e.what());
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 453)         } catch(Base::Exception &e){
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 454)             e.ReportException();
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 455)         } catch (...) {
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 456)             Base::Console().Error("Cannot update representation for '%s'.\n", Obj.getNameInDocument());
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 457)         }
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 458) 
d739a2e4 (jriegel  2013-02-10 20:02:21 +0100 459)         // check for children 
d739a2e4 (jriegel  2013-02-10 20:02:21 +0100 460)         if(viewProvider->getChildRoot()) {
d739a2e4 (jriegel  2013-02-10 20:02:21 +0100 461)             std::vector<App::DocumentObject*> children = viewProvider->claimChildren3D();
d739a2e4 (jriegel  2013-02-10 20:02:21 +0100 462)             SoGroup* childGroup =  viewProvider->getChildRoot();
d739a2e4 (jriegel  2013-02-10 20:02:21 +0100 463) 
d739a2e4 (jriegel  2013-02-10 20:02:21 +0100 464)             // size not the same -> build up the list new
d739a2e4 (jriegel  2013-02-10 20:02:21 +0100 465)             if(childGroup->getNumChildren() != children.size()){
d739a2e4 (jriegel  2013-02-10 20:02:21 +0100 466) 
d739a2e4 (jriegel  2013-02-10 20:02:21 +0100 467)                 childGroup->removeAllChildren();
d739a2e4 (jriegel  2013-02-10 20:02:21 +0100 468)             
d739a2e4 (jriegel  2013-02-10 20:02:21 +0100 469)                 for(std::vector<App::DocumentObject*>::iterator it=children.begin();it!=children.end();++it){
d739a2e4 (jriegel  2013-02-10 20:02:21 +0100 470)                     ViewProvider* ChildViewProvider = getViewProvider(*it);
d739a2e4 (jriegel  2013-02-10 20:02:21 +0100 471)                     if(ChildViewProvider) {
d739a2e4 (jriegel  2013-02-10 20:02:21 +0100 472)                         SoSeparator* childRootNode =  ChildViewProvider->getRoot();
d739a2e4 (jriegel  2013-02-10 20:02:21 +0100 473)                         childGroup->addChild(childRootNode);
d739a2e4 (jriegel  2013-02-10 20:02:21 +0100 474) 
d739a2e4 (jriegel  2013-02-10 20:02:21 +0100 475)                         // cycling to all views of the document to remove the viewprovider from the viewer itself
d739a2e4 (jriegel  2013-02-10 20:02:21 +0100 476)                         for (std::list<Gui::BaseView*>::iterator vIt = d->baseViews.begin();vIt != d->baseViews.end();++vIt) {
d739a2e4 (jriegel  2013-02-10 20:02:21 +0100 477)                             View3DInventor *activeView = dynamic_cast<View3DInventor *>(*vIt);
d739a2e4 (jriegel  2013-02-10 20:02:21 +0100 478)                             if (activeView && viewProvider) {
d739a2e4 (jriegel  2013-02-10 20:02:21 +0100 479)                                 if (d->_pcInEdit == ChildViewProvider)
d739a2e4 (jriegel  2013-02-10 20:02:21 +0100 480)                                     resetEdit();
b3062157 (blobfish 2013-09-22 13:29:09 -0400 481)                               if (activeView->getViewer()->hasViewProvider(ChildViewProvider))
b3062157 (blobfish 2013-09-22 13:29:09 -0400 482)                                 activeView->getViewer()->removeViewProvider(ChildViewProvider);
d739a2e4 (jriegel  2013-02-10 20:02:21 +0100 483)                             }
d739a2e4 (jriegel  2013-02-10 20:02:21 +0100 484)                         }
d739a2e4 (jriegel  2013-02-10 20:02:21 +0100 485)                     }
d739a2e4 (jriegel  2013-02-10 20:02:21 +0100 486)                 }
d739a2e4 (jriegel  2013-02-10 20:02:21 +0100 487)             }
d739a2e4 (jriegel  2013-02-10 20:02:21 +0100 488)         }
d739a2e4 (jriegel  2013-02-10 20:02:21 +0100 489) 
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 490)         if (viewProvider->isDerivedFrom(ViewProviderDocumentObject::getClassTypeId()))
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 491)             signalChangedObject(static_cast<ViewProviderDocumentObject&>(*viewProvider), Prop);
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 492)     }
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 493) 
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 494)     // a property of an object has changed
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 495)     setModified(true);
^120ca87 (wmayer   2011-10-10 13:44:52 +0000 496) }
ickby
Veteran
Posts: 3116
Joined: Wed Oct 05, 2011 7:36 am

Re: small coin error

Post by ickby »

I pushed your fix to my branch. The reason for the error was the following (given as documentation and for interested people):

If a document object get grouped beneath a part, it's inventor node need also be grouped beneath the parts inventor node to allow show/hide for example. Therefore the new code was added by jriegel, it removes the childrens node from the toplevel node in th viewprovider and added it to the parts node. However, he did assume that every children of the part (planes and body) needs to be regrouped every time. But this is not the case, as the planes and the body are added in two steps. In the first step the planes are added and regrouped. That works fine. After that, the body is added to the part and gets regrouped too. But the planes are regrouped again, and as they are already removed from the toplevel inventor node coin gives an error.

Therefore your idea to check if the viewprovider still used by the toplevel node before removing is quite good. Thanks for the hint.
User avatar
tanderson69
Veteran
Posts: 1626
Joined: Thu Feb 18, 2010 1:07 am

Re: small coin error

Post by tanderson69 »

ickby wrote:The reason for the error was the following (given as documentation and for interested people):
thank-you for the explanation.
Post Reply