#619 Made TreeView stable

[NormandC]

I just tested compiling git commit 7901cf7.

First I did

Code: Select all

git checkout 7901cf7ad28952173ec8e2a55c39e8eb5c441e3c .

Then I recompiled FreeCAD.

Unfortunately my FreeCAD version is unchanged and the About FreeCAD window reports the latest commit (0ca5ebe) I had compiled it with after a git pull 4 days ago, even though I had emptied my out-of-source build folder and compiled from scratch.

Since this new compiled version does not exhibit this behaviour, I'm pretty sure I indeed compiled 7901cf7.

This is desired IMO, because visibility of body used for boolean operation does not depend on visibility of the body that contains the Boolean operation. But according to code, Part shouldn't claim the used body as child anymore, so there is likely a bug somewhere.

Sorry, I'm not following you.

This is the issue I am talking about that happens in master after the treeview commits.

Two instances of Body007 can be seen in the tree. IMO this is wrong.

With git commit 7901cf7 just prior to treeview changes, this is what the tree looks like after applying a Boolean fuse with Body007 in active Body.

FC017_grandcross_CM_04.png (39.55 KiB) Viewed 2478 times

There is no duplication of Body007 outside Body, which is exactly as expected.

I was also able to go back to a previous freecad-daily package with the following info:
OS: Ubuntu 16.04.2 LTS
Word size of OS: 64-bit
Word size of FreeCAD: 64-bit
Version: 0.17.10646 (Git)
Build type: None
Branch: (detached from 732bd85)
Hash: 809758b7395f8e591a8a196459bf5a014a864aaf
Python version: 2.7.12
Qt version: 4.8.7
Coin version: 4.0.0a
OCC version: 7.1.0

It is the commit just before 7901cf7, and it too displays the correct nesting behaviour.

But as you can see, Body007 was kept visible under the Boolean feature, and I really can't comprehend how this could be desirable. It is akin to not hiding a PartDesign feature when a new one supersedes it. But this is an unrelated bug.

[realthunder]

Two instances of Body007 can be seen in the tree. IMO this is wrong.

Could you please attach your model file. I'll take a look

[NormandC]

The file is provided in the topic I linked, but I created a new, smaller file that reproduces the issue.

I did further testing, and this happens only to bodies that are both inside a Part container. If there is no Part container, and both bodies are at the root level of the document, then body duplication does not happen when applying a Boolean feature.

Steps to reproduce:

1. Open attached file "FC017_treeview_body_double_instance_bug.fcstd"
2. Make "Body" active
3. Delete Boolean under "Body"
4. Select "Body001"
5. Click on "Boolean operation"
6. Click OK
7. Click on the +/arrow in front of "Boolean" in the Model tree
8. A "Body001" instance is nested under it, its visibility turned on
9. The original "Body001" is still showing under the Part container, on the same level as "Body"
10. Toggling the visibility of either "Body001" instance affects the other.

OS: Ubuntu 16.04.2 LTS
Word size of OS: 64-bit
Word size of FreeCAD: 64-bit
Version: 0.17.10665 (Git)
Build type: None
Branch: master
Hash: 47847513a85ff6615774ef628230f79e37471daf
Python version: 2.7.12
Qt version: 4.8.7
Coin version: 4.0.0a
OCC version: 7.1.0

[realthunder]

The file is provided in the topic I linked, but I created a new, smaller file that reproduces the issue

I check the file, and the Tree View actually behaved correctly. Here is what happen, previously, in the Tree View, one object can have only one corresponding item in the Tree View, even if the object actually belongs to more than one parent. So when an object is claimed by a new parent, like when you create a boolean object, the tree item of object (Body001) is removed from its old parent (Part). Please note that, only the tree item relationship changed, the object 'Part' is still the parent of object 'Body001'. Object 'Boolean' is the second parent of 'Body001'. So, now, 'Part' actually contain two objects with the exact same shape. This is evident when you pre-select some face, and the highlight is not even. See the picture below taken from before my patch,

Screenshot from 2017-04-02 16-37-53.png (267.49 KiB) Viewed 2454 times

So, in other word, the new Tree View actually exposed some problem of the logic in PartDesign.

[NormandC]

I'm sorry but I find your reply confusing.

I check the file, and the Tree View actually behaved correctly.

So you are saying that showing two instances of the Body in the Model tree, one a child of Part, the other a child of Boolean, is correct?

I disagree with that assessment. It is thoroughly confusing.

So, in other word, the new Tree View actually exposed some problem of the logic in PartDesign.

And what would that problem of logic be?

[realthunder]

I'm sorry but I find your reply confusing

The logic you find incorrect lies in PartDesign. Before my patch, the same thing happens, that is, when you create a boolean of Body001, it is NOT removed from 'Part'. So, 'Part' is still effective displaying TWO Body001, you can try on your FC that reverted my change. The preselected face highlight is not even, as shown in my attached picture. This is the result of two identical face in the exact same location.

The old Tree View hide this problem, by removing the Tree Item. The object relationship is still there. The new Tree View reflects the true relationship among the objects. So you should bring your complain to PartDesign of not removing Body001 from Part.

[NormandC]

The preselected face highlight is not even, as shown in my attached picture. This is the result of two identical face in the exact same location.

This is not due to the presence of 2 bodies: it's the result of Body001 not being hidden as it should. Think of a Part Chamfer applied to a part: after the Chamfer, its parent (what you call "child", but IMO it's the other way around) is automatically hidden. This should be the case here.

Toggle the visibility of Body001 off, and you get what should be expected... Only, there are two Body001 when there should be only one in the tree, the one under Boolean.

[realthunder]

This is not due to the presence of 2 bodies: it's the result of Body001 not being hidden as it should. Think of a Part Chamfer applied to a part: after the Chamfer, its parent (what you call "child", but IMO it's the other way around) is automatically hidden. This should be the case here

I can see that the logic does have some problem. You should definitely bring this up to PartDesign, the original thread you posted. But, you can be assured that this is not Tree View's problem, and the developer of PartDesign will understand that.

[DeepSOIC]

Code: Select all

>>> l1=App.ActiveDocument.Part.ViewObject.claimChildren()
>>> for o in l1: print o.Name
... 
PartOrigin
Body
Body001
>>> l2=App.ActiveDocument.Boolean.ViewObject.claimChildren()
>>> for o in l2: print o.Name
... 
Body001
>>> 

Body is displayed in tree exactly according to how children are claimed by Part and Boolean. So no bug in tree view.

[wmayer]

Today I found a major performance regression with the new tree view. In issue #1999 you will find the file FreeCadProblem.step which takes 15 minutes to load in release mode. For testing purposes and to be sure it's caused by this PR I reverted all recent changes of Tree.cpp and Tree.h and then the load time was OK again -- which is 30 seconds.

When loading the STEP file then from the importer this block in ImportOCAF.cpp seems to take forever (in debug mode):

Code: Select all

        if (!localValue.empty()) {
            pcPart = static_cast<App::Part*>(doc->addObject("App::Part",name.c_str()));

            // localValue contain the object that I must add to the local Part
            // I must add the PartOrigin and the Part itself
            for (std::size_t i=0; i<localValue.size(); i++) {
                pcPart->addObject(localValue[i]);
            }

            lValue.push_back(pcPart);
        }