"Robust References" Redux

Here's the place for discussion related to coding in FreeCAD, C++ or Python. Design, interfaces and structures.
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
ezzieyguywuf
Posts: 656
Joined: Tue May 19, 2015 1:11 am

Re: "Robust References" Redux

Post by ezzieyguywuf »

ickby wrote:I think you are one of only very few people around the globe that understand that framework :)
Lol, you are too kind.
ickby wrote:That is not exactly true. Basically all TopoShape functions internally use BRepBuilderAPI_MakeShape or derived functions.
You are correct that TopoShape itself uses BRepBuilderAPI_MakeShape and its derivatives. But unless I'm missing something, things such as Part::Box (defined in FeaturePartBox.h) do not use these TopoShape methods. Instead, they create the TopoDS_Shape themselves on creation/rebuild. So, for example, if I make a 10x10x10 cube, then Part::Box::execute() is called with height/length/width set to 10. If the user modifies the height to 20, rather than calling an occ function that implements Generated/Modified/Deleted, we just create a brand new TopoDS_Shape with the new height and call it a day.

That being the case, I was thinking that probably the correct way to address this for primitives is to simply extend the implementation such that the primitive 'remembers' its last dimensions (so height/length/width in the case of a cube) and when a regenerate() is called, it can compare the current values to the previous values. If height changed, I know which faces are Modified, etc.
ickby wrote:Again, I'm not sure why you do not build up the history automatically in TopoShape. Is there any good reason to let it be done in the calling code everytime again and again for every method call to TopoShape?
I'm not sure I understand 100% what you are saying here, but I'll use Part::Fillet as an example as to why I am doing things the way that I am. By all means, I am open to feedback/criticism, I'm just not sure I exactly understand what you are trying to say, so perhaps this will help progress the conversation:

1) User creates a Cube (Part::Box is instantiated and stored as the Cube Document Object)
2) User Fillets an Edge on the Cube (Part::Fillet is instantiated with Cube as the Base Shape, and stored as Fillet Document Object)
3) User changes Height on Cube (Part::Box is regenerated with new height, then Part::Fillet is regenerated with new Base Shape)

Part::Fillet needs to keep track of the Base Shape and any changes in its Topological Evolution. So, when the Fillet is first created, it knows what Base Shape is and that all the Faces are "Generated". When Fillet::Box is regenerated, it needs to know which faces in the new Part::Box are Modified/Deleted/Generated etc. (i.e. there may be Generated Faces if the Base Shape had a pocket cut out of it).

The way that Part::Box is currently implemented, though, it doesn't really utilize TopoShape except to store the created TopoDS_Shape. My idea is to extend TopoShape with methods such as ChangeCubeHeight, ChangeCubeWidth, etc... and make the appropriate calls from Part::Box. This should store sufficient information in the Part::Box TopoShape so that when Part::Fillet gets regenerated, it knows what's going on and can still find the Edge it's looking for.

Writing it all out, it sounds pretty confusing to me...So let me know if that doesn't make sense. I feel like what you're trying to say will probably simplify things, I just can't wrap my head around it.
ickby wrote:Definitely awesome work! Thanks for going on with this!
Hey thanks for your nice words, they really help me to stay motivated and to keep working on this!
cox
Posts: 971
Joined: Wed Nov 26, 2014 11:37 pm

Re: "Robust References" Redux

Post by cox »

Thanks again for your efforts, still enjoy your progress reports a lot :D
Need help? Feel free to ask, but please read the guidelines first
User avatar
DeepSOIC
Veteran
Posts: 7896
Joined: Fri Aug 29, 2014 12:45 am
Location: used to be Saint-Petersburg, Russia

Re: "Robust References" Redux

Post by DeepSOIC »

ezzieyguywuf wrote:My idea is to extend TopoShape with methods such as ChangeCubeHeight, ChangeCubeWidth, etc... and make the appropriate calls from Part::Box.
Sounds like no good idea to me. This will require adding many many methods for each primitive-like feature (e.g. Draft Ellipse; Sketch; features from add-on workbenches).
ezzieyguywuf wrote:When Fillet::Box is regenerated, it needs to know which faces in the new Part::Box are Modified/Deleted/Generated etc. (i.e. there may be Generated Faces if the Base Shape had a pocket cut out of it).
I have used "Modified" method to establish correspondence between source and result shapes of GFA. See https://github.com/FreeCAD/FreeCAD/pull ... 088f9R1542
The problem is, I don't understand the exact purpose of "Modified" method, and I feel like it may collide with your toponaming. If you have some understanding of it, can you explain, what 'Modified' is supposed to do exactly, is it OK to use it for the purpose I used it for? (I need to find out, which pieces in GFA result came from which input shape ("Argument"), and 'Modified' seems to provide exactly the info I need).
ezzieyguywuf
Posts: 656
Joined: Tue May 19, 2015 1:11 am

Re: "Robust References" Redux

Post by ezzieyguywuf »

DeepSOIC wrote:Sounds like no good idea to me. This will require adding many many methods for each primitive-like feature (e.g. Draft Ellipse; Sketch; features from add-on workbenches).

What's wrong with adding methods as appropriate to TopShape?
DeepSOIC wrote:The problem is, I don't understand the exact purpose of "Modified" method, and I feel like it may collide with your toponaming. If you have some understanding of it, can you explain, what 'Modified' is supposed to do exactly, is it OK to use it for the purpose I used it for? (I need to find out, which pieces in GFA result came from which input shape ("Argument"), and 'Modified' seems to provide exactly the info I need).
If you check the tnaming sample in the occ source, you'll see them using Modified and friends. I haven't delved into the underlying code too much, but from what I can tell Modified, is implementation Dependant in the BRepAlgoAPI_MakeShape descendant (bc it's a virtual function and not defined in BRepBuilderAPI_MakeShape). For example, for BRepFilletAPI_MakeFillet it returns a list of modified faces from the OldShape.

I don't think your use should collide with mine. We're both merely using it to get data about the operation, and storing that information.
User avatar
DeepSOIC
Veteran
Posts: 7896
Joined: Fri Aug 29, 2014 12:45 am
Location: used to be Saint-Petersburg, Russia

Re: "Robust References" Redux

Post by DeepSOIC »

ezzieyguywuf wrote:I don't think your use should collide with mine. We're both merely using it to get data about the operation, and storing that information.
OK, thanks for reply.
ickby
Veteran
Posts: 3116
Joined: Wed Oct 05, 2011 7:36 am

Re: "Robust References" Redux

Post by ickby »

I'm also not entirly sure if I get you correctly, this is complicated stuff and I do my best to stay on track, but well, I may miss many things :) I also do not know how occ tnaming works, so many of the following can be wrong for the usage of this framework.
ezzieyguywuf wrote: You are correct that TopoShape itself uses BRepBuilderAPI_MakeShape and its derivatives. But unless I'm missing something, things such as Part::Box (defined in FeaturePartBox.h) do not use these TopoShape methods. Instead, they create the TopoDS_Shape themselves on creation/rebuild.
Ah yes, that is true. This makes it of course harder for history building. If you remember our very first discussions about this stuff there was a point to move all calls to occ to TopoShape. The box would be such a example. One would need to add a "makeBox(...)" call in the TopoShape class. Than internally history building would be easy with the "topFace" etc. methods which directly identify all faces. Does this not suffice? For the first part in a history I think it should be enough to have all faces be consistant for every recompute, e.g. top face always needs to be identified as top face and not suddenly be at the bottom. There is no need for knowing what was changed as everything is "Generated" always, for the very first run and for all others too.
1) User creates a Cube (Part::Box is instantiated and stored as the Cube Document Object)
2) User Fillets an Edge on the Cube (Part::Fillet is instantiated with Cube as the Base Shape, and stored as Fillet Document Object)
3) User changes Height on Cube (Part::Box is regenerated with new height, then Part::Fillet is regenerated with new Base Shape)

Part::Fillet needs to keep track of the Base Shape and any changes in its Topological Evolution. So, when the Fillet is first created, it knows what Base Shape is and that all the Faces are "Generated". When Part::Box is regenerated, it needs to know which faces in the new Part::Box are Modified/Deleted/Generated etc. (i.e. there may be Generated Faces if the Base Shape had a pocket cut out of it).
Is this like that? Why does the very first part need to be anything but "Generated"? If there is nothing before the box in the history all faces of the box are new, hence they are generated. Even if changed this modeling step only creates new faces. The only important thing is that the identifiers always identify the exact same face, e.g. that the identifier "top face" always get the top face and not the bottom one. This can be ensured with the respective MakeBox functions.

I agree that for example a pocket needs to know what was generated/modiefied etc. as it needs to remap faces that already existed to the correct identifiers which have been used before. But this can happen inside the TopoShape class when "pocket(...)" is called as there the respective methods of occ are available.

The way that Part::Box is currently implemented, though, it doesn't really utilize TopoShape except to store the created TopoDS_Shape. My idea is to extend TopoShape with methods such as ChangeCubeHeight, ChangeCubeWidth, etc... and make the appropriate calls from Part::Box. This should store sufficient information in the Part::Box TopoShape so that when Part::Fillet gets regenerated, it knows what's going on and can still find the Edge it's looking for.
IMHO in your example Fillet does not need to know what was going on before, it must only know the identifier of the edge it should fillet. Then the box and pocket operations before are responsible that the identifiers stay consitent so that fillet graps the correct one. I imagine it like this:
recompute.png
recompute.png (66.97 KiB) Viewed 2504 times
Each DocumentObject builds a output shape and ensures that the identifiers are correctly assigned to faces. So all box needs to do is to make sure that "TopFace" identifier always refere to the top face.
For pocket it is harder. After chaning the shape it first needs to make sure that all old identifiers are correctly assigned to the correct faces, for exampel that the identifier "TopFace" still points to the same face as it did in the Box shape before the pocket operation. After it ensured this it must give new identifiers to the new created faces. When recomputed it must ensure that the new created faces get the same identifiers as before, for example the bottom face of the pocket always needs to get the same identifier "BottomFace" (except it is a through all pocket, than this identifier should not be used). This relating of identifier to subshape must be fully done by Pocket.
Fillet than has the same problem as Pocket: After chaning the shape it must ensure that the identifiers used before still point to the very same subshapes. Afterwards all newly created faces must get an identifier, and on recompute those identifiers must be consitently assigned so that the same face from user view always gets the same identifier.

In this szenario each step only needs to know what it has done to the shape itself, not what was done before. Hence it is enough to know what subshape was Modified/Generated/Deleted in the algorithm used. From this point of view it is best to move all the history building and identifier-to-subshape building directly into TopoShape, as there you have access to Modified/Generated/Deleted when needed.


Hope this makes some sense, and if I only tell what you already know please kindly ignore it :)
ezzieyguywuf
Posts: 656
Joined: Tue May 19, 2015 1:11 am

Re: "Robust References" Redux

Post by ezzieyguywuf »

ickby wrote:I'm also not entirly sure if I get you correctly, this is complicated stuff and I do my best to stay on track, but well, I may miss many things :) I also do not know how occ tnaming works, so many of the following can be wrong for the usage of this framework.
I'm also probably a bit to blame here, I don't think I've done a great job trying to explain myself and I haven't really discussed how the TNaming stuff works too in-depth either. I should probably go back and update my "How TNaming works" document, since I've learned a bit since I wrote it
ickby' wrote:Ah yes, that is true. This makes it of course harder for history building. If you remember our very first discussions about this stuff there was a point to move all calls to occ to TopoShape. The box would be such a example. One would need to add a "makeBox(...)" call in the TopoShape class.
Yes, I do remember this from our earlier conversations. I think perhaps I got a bit too wrapped up in trying to just get something that works, and sort of lost track of this direction. Actually, I think what tripped me up the most was the use of the PropertyPartShape to store the TopoShape in each Part::Feature. For some reason, in my head, since PropertyPartShape::getShape returns a const TopoShape, I thought I needed to add access methods such as PropertyPartShape::makeBox etc. But really, I can just do something like:

Code: Select all

TopoShape ModifiedShape = PropertyPartShape::getShape() // pseudocode
ModifiedShape.makeBox(length, width, height);
//or
//ModifiedShape.modifyBox(length, width, height);
PropertyPartShape::setShape(ModifiedShape) // pseudocode
So yea, perhaps I should re-examine adding all the methods for topo stuff straight to TopoShape...
ickby wrote:Than internally history building would be easy with the "topFace" etc. methods which directly identify all faces. Does this not suffice? For the first part in a history I think it should be enough to have all faces be consistant for every recompute, e.g. top face always needs to be identified as top face and not suddenly be at the bottom. There is no need for knowing what was changed as everything is "Generated" always, for the very first run and for all others too.
*snip*
Is this like that? Why does the very first part need to be anything but "Generated"?
This does suffice for the initial creation of the Box, but what happens when the box is 'modified'? I don't think we can rely on the Box always having all the faces to be 'Generated'. Your example with the Box->Pocket->Fillet is a great one. The Fillet operation has to know about one specific Edge. If the Box itself is changed (let's say the height is adjusted), how does it still know about that Edge? The way that the TNaming algorithm works, it has to know which faces on the Box changed. Then, it can re-compute the reference to the Edge. If instead of tracking the modification to the Box we instead just Generate a brand new box, then the TNaming algorithm can no-longer re-compute the reference to the appropriate Edge because it doesn't have enough information.
ickby wrote:IMHO in your example Fillet does not need to know what was going on before, it must only know the identifier of the edge it should fillet. Then the box and pocket operations before are responsible that the identifiers stay consitent so that fillet graps the correct one.
Hm, so let me explain a little bit about how I am using TNaming here. That may clear things up a bit.

Code: Select all

0 Root Node
|-0:1 Selection Node
   |-0:1:1 Selected Edge
       |- 0:1:1:1 Context Shape
|
|-0:2 Generated Cube
   |-0:2:1 Generated Face (Top)
   |-0:2:2 Generated Face (Bot)
   |.....etc.....
   |-0:2:6 Generated Face (Back)
|
|-0:3 Modified Cube (pocket operation)
   |-0:2:1 Modified Faces (Top face if pocket is on top face)
   |-0:2:2 Generated Edges
   |-0:2:3 Generated Faces
   |-0:2:4 Deleted Faces (none in this example)
|
|-0:4 Filleted Cube
   |-0:4:1 Modified Faces
   |-0:4:2 Generated Faces
   |-0:4:3 Generated Edges
   |-0:4:4 Deleted Faces
Ok, hopefully this makes a bit of sense. This tree is meant to represent the "Data Framework" that is created in order for OCC's TopoNaming to work. This is currently done in the TopoNamingHelper class that I've defined. Whenever the fillet operation occurs (so, before the 0:4 node is created), it first 'Selects' an Edge. This stores in the Data Framework a reference to the Edge as well as a "context shape" that the Edge is found within. I can now easily get a reference to that Edge back by using the '0:1:1' Tag (this is all OCC Tnaming stuff). But what happens when the Box changes? Well, I think what you're suggesting is to just replace the '0:2' node with the new Box, keep all the faces as 'Generated', and call it a day. Unfortunately, this will break OCC's ability to resolve the '0:1:1' Tag back to the appropriate Edge, because it does not have enough information. All it knows it that the Edge used to be inside Box1, and it will return that same edge. But since you're now trying to fillet an Edge on Box2, you'll get an OCC error "No suitable edge to fillet or chambfer". Instead, you must do something like this:

Code: Select all

0 Root Node
|-0:1 Selection Node
   |-0:1:1 Selected Edge
       |- 0:1:1:1 Context Shape
|
|-0:2 Generated Cube
   |-0:2:1 Generated Face (Top)
   |-0:2:2 Generated Face (Bot)
   |.....etc.....
   |-0:2:6 Generated Face (Back)
|
|-0:3 Modified Cube (pocket operation)
   |-0:2:1 Modified Faces (Top face if pocket is on top face)
   |-0:2:2 Generated Edges
   |-0:2:3 Generated Faces
   |-0:2:4 Deleted Faces (none in this example)
|
|-0:4 Filleted Cube
   |-0:4:1 Modified Faces
   |-0:4:2 Generated Faces
   |-0:4:3 Generated Edges
   |-0:4:4 Deleted Faces
|
|-0:5 Modified Cube (resized)
   |-0:5:1 Modified Faces (Top face if height is changed)
   |-0:5:2 Generated Edges (none)
   |-0:5:3 Generated Faces (none)
   |-0:5:4 Deleted Faces (none)
|
|-0:6 Re-Filleted Cube
   |-0:6:1 Modified Faces
   |-0:6:2 Generated Faces
   |-0:6:3 Generated Edges
   |-0:6:4 Deleted Faces
So here, the '0:5' describes how the original cube in '0:2' was modified, and gives OCC enough information to retrieve the appropriate Edge for us when we use the '0:1:1' tag to grab it. Now, when I recompute my Fillet feature, even though I have to re-compute the pocket as well (since it was on the Top face, which moved), OCC still has enough information to figure out which Edge I'm asking about.
ickby wrote:I imagine it like this:
*snip*
Each DocumentObject builds a output shape and ensures that the identifiers are correctly assigned to faces. So all box needs to do is to make sure that "TopFace" identifier always refere to the top face.
Hrm, so I guess what you are suggesting is to create a wrapper around the OCC TNaming/Data Framework stuff. Maybe, for example, we could say that for a Box, the ...:1 sub-node is always the Top Face, the ...:2 sub-node is always the Bottom Face, etc. etc... Then, whenever the next operation is called, it would just refer to the appropriate sub-node.

Is that scalable though? What happens when we're not dealing with a simple primitive, such as a Box or Cylinder? How will we properly keep track of what node refers to what?

The way I'm doing it for Part::Fillet is as follows: I've added a std::string to FiletBase that holds the Tag for the Selected Edge. So, I still use the integer as we do today, but I only use it to figure out which Edge the User wants to select. Then, I use the OCC TNaming Data Framework to store a reference to that Edge, and then keep the Tag that points back to that reference, '0:1:1' in our example. This easily scales for any shape, as I assume I can do something similar for any geometry that is selected.
ickby wrote:Hope this makes some sense, and if I only tell what you already know please kindly ignore it :)
It does make sense, and thank you for the thoughtful and challenging post! Please let me know if my responses make any sense to you. I welcome the ability to bounce these ideas around, and if the approach I am taking can be improved/simplified I am all ears. Please don't take my response here as defensive, I am merely trying to explain what I've done and why I've done it. If we can modify my approach to work more similar to what you've laid out, I think that would be best. Unfortunately, I cannot see how that will work. But maybe I am just too close to the problem and could use the help of some outside perspective.
ickby
Veteran
Posts: 3116
Joined: Wed Oct 05, 2011 7:36 am

Re: "Robust References" Redux

Post by ickby »

Oh I see, occ records every change as a new entry in the history and on resolving a reference they try to "go back" through all changes. As you already noted I had a different way of working in my mind. I thought that every operation gets one entry in the history, and on rebuild it just changes the entry. So that references are like 1:2:1 and the history is responsible that the numbers are always point to the exact same shape.

Now of course I can#t say which method is better. Actually mine is only in my head, never tested, so most likely occ method is better :) But now I see your trouble implementation wise, of course recording absolutely every step in a single history for all shapes is not that trival. How do you resolve the sharing of a history by multiple TopoShapes? Do you have a global datastructure where histories are stored and shared for the TopoShapes that are connected?
ezzieyguywuf
Posts: 656
Joined: Tue May 19, 2015 1:11 am

Re: "Robust References" Redux

Post by ezzieyguywuf »

ickby wrote:How do you resolve the sharing of a history by multiple TopoShapes? Do you have a global datastructure where histories are stored and shared for the TopoShapes that are connected?
Right now, I don't. And actually, the more I thought about it, I don't really need to. I don't need to because of the way that FreeCAD works: it creates and maintains its own TopoShape for each Part::Feature. This TopoShape gets updated whenever it gets changed, and then any Part::Feature that depends on this Part::Feature gets recomputed.

Since that's the case, I can make sure that each Part::Feature maintains its own Topological History. Any subsequent Feature that depends on it doesn't need to know the whole history of the child Feature. It just needs to know the final shape. It will add this final shape to its own Topological History as "Generated". If the child Feature changes and this dependent Feature is recomputing, it will add another node to its Topological History as "Modified" of the original.

It may be non-trivial to figure out which faces were Modified/Deleted/Generated compared to the original, but I'll deal with that later. Right now I'm figuring it out for primitives and so far that seems pretty straightforward.

Also, this may not be as straightforward for, say, MultiFuse, where there are multiple child Features. Right now, I just have the MultiFuse take the Topological History from the first in the list of ShapesToFuse and it just discards the rest. That's on my list of "things to figure out". But again, like I've explained above, even in the MultiFuse case, maybe it doesn't need the full history of all the children...I just haven't gone back to that yet.
ezzieyguywuf
Posts: 656
Joined: Tue May 19, 2015 1:11 am

Re: "Robust References" Redux

Post by ezzieyguywuf »

Alright, here is another update. I've figured out how to track the Modified/Deleted/Generated faces, I think. For any primitive, all you need to know is two things: (1) The type of primitive, and (2) what changed on the primitive. As it stands, each Part::Feature that implements a primitive (i.e. Part::Box) already has all that info on-hand, so I am now just using that information to figure out which Faces are Modified. There shouldn't ever be a Deleted/Generated Face if a Primitive is being updated - that can only happen if a Part::Cut or Part::Fillet or something else is created with the Primitive as the base shape.

You can look at the commit history on the tnaming_ezziey branch on my github repo for a bit more info. I would start with commit "b3b0178555546d6f0e281d9ca558cbdba1f27893". Mostly what I've changed since my last update on this thread is that I'm now letting each Part::Feature manage its own Topological History and only sharing/copying topological history as needed. This is per ickby's suggestion, which was a great one. Let me explain a bit. If you want to see the TopoNamingHelper in action, check out 'createBox', 'updateBox', 'createFilletBaseShape', 'createFillet', and 'updateFillet' in TopoShape.

So, the way the topological history is tracked is in a hierarchical tree. Node "0" is the root node of the tree. Node "0:1" I have designated will always hold a list of any selected edges. So, for example, if you 'Select' one edge, it will be stored in "0:1:1", the second Edge you select will be stored in "0:1:2", etc. The rest of the hierarchy will be specific to whatever Part::Feature is being made, with one exception: the 'tip' node, i.e. "0:n" where n>1 and n == NumberOfChildren("0"), must always have a child which represents the latest TopoDS_Shape of the Part::Feature that is being tracked.

So, let's used a Part::Fillet as an example. The "0:1" branch will hold the 'Select'ed edges, for later retrieval without the use of 'TopExp::MapShapes'. This is the key that allows us to have a 'robust reference' to filleted edges.

The "0:2" branch will hold the topological history of BaseShape. So, if the BaseShape is a Part::Box, then "0:2:1" will hold the initial box (let's say it's a 10x10x10) while any subsequent changes to the box will be tracked on nodes "0:2:n". Each "0:2:n" will have as a child a node that stores the Modified Faces "0:2:n:1", and then under that node will be each of the actual modified faces "0:2:n:1:1", "0:2:n:1:2", etc... Try not to get too caught up on this hierarchy stuff, it's all managed by the TopoNamingHelper, so future Part::Feature devs shouldn't have to mess with it.

The "0:3" branch in our fillet example will contain the actual filleted shape. So, "0:2:1" will be the initial filleted shape with "0:2:n" representing any changes to that shape. Why do we need to track each change to the filleted shape? Well, let's assume that this filleted shape is used as the BaseShape for another Part::Feature, let's even say it's another Part::Fillet. In this case, the "0:2" branch will be copied from the "0:3" branch of the BaseShape Part::Fillet, since that's at the tip of the tree. I needed to do that so that I could have the "0:2" branch available for BaseShape, or FuseShapes in the case of a multifuse etc.

So, going back to the main changes since my last update, creating this tree/branch structure was one of them. Another one is that I created a method in TopoNamingHelper for copying a branch from one TopoNamingHelper to another. This is necessary in cases like Part::Fillet where the topology is dependent on a BaseShape. Rather than having Part::Fillet trying to figure out what changed in its BaseShape, it can just steal the history from the BaseShape itself.

I'm running into an issue I can't figure out though - I can get it to work properly when I test it outside of FreeCAD, i.e. using TopoNamingHelper in a bare-bones 'FakeTopoShape' class that I created. But when its incorporated into FreeCAD I run into issues. I'm going to post a different thread with an explanation of what's going on to see if anyone can help me.

Either way, figuring out how to copy the topological history from one place to another is a big step, as well as starting to hone in on the best hierarchical structure to use in the topological history.
Post Reply