PR #4752 Topological Naming

[Zolko]

The new topo names are stored as a string table inside each shape (Part::TopoShape) that maps the encoded name to index. Internally, we still need the index name to access OCC sub-shapes.

OK it begins to make sense. So, how do objects reference elements now: by the (old) index names or the (new) mapped names ? For example, when I attach an LCS to elements of a sketch, I can see that the references that are displayed are the (old) index names. Is that only for display ?

When I attach an LCS to a corner of a sketch, I see the following in the Document.xml file (inside the FCStd) :

Code: Select all

<Property name="Support" type="App::PropertyLinkSubList">
    <LinkSubList count="3">
    <Link obj="Sketch_Part_1" sub="Vertex1" shadow=";g1v1;SKT.Vertex1"/>
    <Link obj="Sketch_Part_1" sub="Edge1" shadow=";g1;SKT.Edge1"/>
    <Link obj="Sketch_Part_1" sub="Edge5" shadow=";g4;SKT.Edge5"/>
    </LinkSubList>
</Property>

and when I change the sketch by deleting and adding lines, I get:

Code: Select all

<Property name="Support" type="App::PropertyLinkSubList">
    <LinkSubList count="3">
    <Link obj="Sketch_Part_1" sub="Vertex3" shadow=";g1v1;SKT.Vertex3"/>
    <Link obj="Sketch_Part_1" sub="Edge3" shadow=";g1;SKT.Edge3"/>
    <Link obj="Sketch_Part_1" sub="Edge2" shadow=";g4;SKT.Edge2"/>
    </LinkSubList>
</Property>

So the index names changed, but the mapped names didn't. Therefore, I guess that this (your -asm3 branch) does solve the toponaming problem (for sketches at least, and it's what I care about the most). Congratulations !

Now, this also changes the data structure of FreeCAD files, therefore I think we should also be careful about what is displayed and saved. May I challenge the "for compatibility reasons" ? Displaying a secondary info (the varying index name) instead of the really important info (the stable mapped name) might seem like a good idea at first glance, but it might bite us in the future.


EDIT: when I open the same file with main FreeCAD, I read the following in Document.xml:

Code: Select all

<Property name="Support" type="App::PropertyLinkSubList">
    <LinkSubList count="3">
        <Link obj="Sketch_Part_1" sub="Vertex3"/>
        <Link obj="Sketch_Part_1" sub="Edge3"/>
        <Link obj="Sketch_Part_1" sub="Edge2"/>
    </LinkSubList>
</Property>

[wsteffe]

See here for an explanation of how the recursive encoding using shape history works.

I have looked at. I may undersatand the intent but it is quite difficult to fully grasp the logic of name enconding.

The heuristics lies in the fact that one can often find more than one match, and I usually just pick the first one.

Here the match should refer to the "search the current (modified) shape for elements that can be traced back to the same geometry element".
Is it true ?
This search is done in the last shape (after the history has been fully regenerated). On the other side the last matching of a named geometry in the recursive decoding could be associated with a previous state in the history. So when there is "more than one match" does it mean that more than one element of the same type was derived from the last mapped common ancestor ?

[realthunder]

OK it begins to make sense. So, how do objects reference elements now: by the (old) index names or the (new) mapped names ? For example, when I attach an LCS to elements of a sketch, I can see that the references that are displayed are the (old) index names. Is that only for display ?

Part::TopoShape has API to allow accessing sub shape with both the index name or the mapped name. But internal to TopoShape, it will always translate the name to index because that's what OCC understands. For programmer, as long as the sub shape name is retrieved from a link type property before accessing (i.e. not stored in some user data structure or other property like a PropertyString), then it can be trusted.

Now, this also changes the data structure of FreeCAD files, therefore I think we should also be careful about what is displayed and saved. May I challenge the "for compatibility reasons" ? Displaying a secondary info (the varying index name) instead of the really important info (the stable mapped name) might seem like a good idea at first glance, but it might bite us in the future.

The displayed name is for human to read, but the mapped names are really not for that. You may find very similar name with maybe only one character change, or you may find a name with hundreds of characters long, which are obviously not suitable for display. What's important is how the program uses the name.

[realthunder]

Here the match should refer to the "search the current (modified) shape for elements that can be traced back to the same geometry element".
Is it true ?

A match requires two parts. The first part is from the mapped element name stored in some link type property that is now missing in the modified shape. So we perform recursive decoding to that element name to trace to some element in the ancestor shape. The second part is the elements in the current modified shape. Decode those element names and try to find one that can be traced back to the same element in the same ancestor shape.

So when there is "more than one match" does it mean that more than one element of the same type was derived from the last mapped common ancestor ?

Yes, that is correct. Depending on the extend of model changes, and how far back into the history, it is common to get many matches.

[wsteffe]

Hello RT, thanks for the clarification.

[xryl669]

I was wondering, from Zoko's post and the numerous similar remark made by other, why can't we do this:

1. When a change is made to a sketch
2. Check if the following functions still applies (if it does, we're done)
3. Else, we know where the function is failing (since it's reported in the error console), we still have the previous geometry (have we?)
4. So, can't we find out the failing edge/face/vertex in the previous geometry and perform a 3D lookup for a similar edge/face/vertex in the new updated geometry ?
5. If the item to look for is an edge, we might want to accept any side that's located along the same direction and offset, or that's starting/ending from the same vertex, or that's connecting the same face
6. If the item to look for is a face, we might want to accept any plane with the same normal and the closest position (even if the sides' count is different?)
7. If the item is a vertex, we might select the closest vertex

What I've described above is my "mental" algorithm when I need to fix for TNI that I'm applying. If applied in Zoko's example above with the 2 extruded rectangles, it'll work for all the cases except the one where the face is missing (obviously).

Please notice that realthunder's branch solves it differently, but they are case where it'll fail as well, for example, if you delete a line in a polyline sketch and recreate a line somewhere else and constraint its 2 end points to be locked on the deleted's line end point. In that case, there is no ID link anymore to map the previous (deleted) line to the new line, since the end point are not the same, so nothing except a 3D/2D lookup can solve it.

[chrisb]

What I've described above is my "mental" algorithm when I need to fix for TNI that I'm applying.

Your algorithm works only if an error is involved, but many if not most problems concerning toponaming don't raise an error, they just change the model in an unwanted way.

Beyond that, your algorithm shows that it always needs some heuristics such as searching the face with the normal close to the normal before the change. A problem is already to decide when a special mapping algorithm has to be applied. To show the problem let's make another mental experiment:
Imagine a cube with something attached to its side. Now turn the cube by 90°. I would expect that the attached object is turned as well. However, if you search the face with the closest normal, you will find a different one and change the attachment unwanted.

[xryl669]

I'm not sure it applies to your example, since in that case, there would be no breakage at first (the initial edge still exists after the rotation, so there's no need to run the algorithm).

As for changing the model in an unwanted way, I completely agree with you. My algorithm is not perfect and should only be applied after realthunder's operation upon an error. Without such algorithm, the user would have to fix the result anyway, so if my algorithm solves only half of this cases, it's still an improvement in terms of UX. If the algorithm breaks the user expectation, she'll have to fix it as if the algorithm wasn't applied so it's not an issue IMHO.

Most of the time, changing a parameters involves modifying a length without actually modifying the topology, so it'll fit the work correctly.
Just take the example I've described above, about adding a line and constraining it to fit between a removed line. Realthunder's algorithm will fail here and my algorithm will fix it.

[adrianinsaval]

Just take the example I've described above, about adding a line and constraining it to fit between a removed line. Realthunder's algorithm will fail here and my algorithm will fix it.

but it works, try it out, I think realthunder already implemented something similar to what you describe

[Pauvres_honteux]

A simple model showing how to trigger the TopoNaming-tormentor:

PartDesign_RealThunder_2021_717_24301_Sketch_profile_move_vs_radius_connection.FCStd

(201.93 KiB) Downloaded 97 times

One way of triggering it:

Sketch_attachment_point_move.png (52.56 KiB) Viewed 4768 times

Another way of triggering it:

Sketch_profile_center_point_move.png (59.21 KiB) Viewed 4768 times

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


How to manually fix it:

Right click on ?Edge23 and select "Delete".

Edge_23_lost_track_of_its_underlying_curve_id.png (72.85 KiB) Viewed 4768 times

Click on the new intersection Edge3 in the 3D-view.

Edge_3_reconnected_to_its_underlying_curve_id.png (85.36 KiB) Viewed 4768 times

Done with
OS: openSUSE Leap 15.2 (KDE//usr/share/xsessions/default)
Word size of OS: 64-bit
Word size of FreeCAD: 64-bit
Version: 2021.717.24301 +3842 (Git) AppImage
Build type: Release
Branch: LinkDaily
Hash: 44be6d539871be4083e83f428e8fcfbcf38408ef
Python version: 3.9.6
Qt version: 5.12.9
Coin version: 4.0.1
OCC version: 7.5.2


As a user I would expect the underlying TopoNaming workings to sort out this type of id-change as far as it is possible.
If TopoNaming workings fails, it would be preferable if some sort of window pops up showing the old and new intersecting curve asking user if the new one is ok to use instead. With an ok- and preview-button, where the ok-button is preselected. That way the user only needs to confirm by pressing the Enter-key. The recalculation then continues down the tree and the process repeats with minimal effort / fast workflow from user side.