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.