Topological Naming, My Take

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!
logari81
Posts: 658
Joined: Mon Jun 14, 2010 6:00 pm

Re: Topological Naming, My Take

Post by logari81 »

adrianinsaval wrote: Wed Oct 05, 2022 6:16 pm
logari81 wrote: Wed Oct 05, 2022 1:53 pm Sorry guys, you have hijacked someone else's thread, I think the moderators should split irrelevant discussions, I come to this thread to look for an update about the merging of realthunder's implementation.
Discussions about toponaming aren't off topic. There's probably going to be an announcement soon for the first official beta, stay tuned but not necessarily to this topic.
ok I see now that the discussion has turned into identifying deficiencies of RT's solution, which I agree is relevant, and this is what I had actually asked about before:
logari81 wrote: Thu Aug 11, 2022 9:17 am Is there at any point a list of issues with RealThunder's implementation that prevent merging it?

I have read RealThunder's description of his solution and looks absolutely convincing to me.
Now my next question is, have you tried/managed to communicate the case(s) that is(are) apparently failing to RT and get his opinion on the issue?
chrisb
Veteran
Posts: 53922
Joined: Tue Mar 17, 2015 9:14 am

Re: Topological Naming, My Take

Post by chrisb »

logari81 wrote: Wed Oct 05, 2022 1:53 pm Sorry guys, you have hijacked someone else's thread, I think the moderators should split irrelevant discussions, I come to this thread to look for an update about the merging of realthunder's implementation.
You are right! Alas, it's a hassle to get this sorted. This topic started as an announcment of Realthunder, and we definitely need a place where he presents his work, and also a place where where to discuss it. There are several topics now and I am unsure how to continue.

A milestone for the next release is the integration of the TNP stuff into master. I have already created a subforum "Toponaming and Link Branch" for discussing this, and maybe we should create another topic in there, where @realthunder can post his progress. I can then pin that topic and I could also try to lock it, but I'm not sure, if the author than can still edit his own first post.
A Sketcher Lecture with in-depth information is available in English, auf Deutsch, en français, en español.
realthunder
Veteran
Posts: 2190
Joined: Tue Jan 03, 2017 10:55 am

Re: Topological Naming, My Take

Post by realthunder »

Zolko wrote: Thu Oct 06, 2022 5:38 am
C_h_o_p_i_n wrote: Fri Sep 30, 2022 11:12 am if too many chain-elements inbetween are deleted
Too many ? We're talking about 3 simple consecutive PartDesign fonctions where 1 is deleted in the middle, and which doesn't pose any problems in FreeCAD 0.20

So i think it's quite normal that a manually reatachment of any orhphaned leaves/twigs might sometime be neccessary.
But then, what's the avantage of all this new code ? FreeCAD v0.20 cannot deal with insertions, LinkDaily cannot deal with deletions.
Missing this post some how. Too busy recently. Anyway, the problem you discovered is due a bug introduced by a recent change in Jun. The problem is not really about topo naming. Because when a feature is deleted, the core behavior (both my branch and upstream master) is to break all link to this feature. So the sketch is effectively detached. The bug is caused by the AttachExtension reseting the placement before calculating the new position. The fix is simple, just restore the placement on abort or error. See here.
Try Assembly3 with my custom build of FreeCAD at here.
And if you'd like to show your support, you can donate through patreon, liberapay, or paypal
User avatar
adrianinsaval
Veteran
Posts: 5541
Joined: Thu Apr 05, 2018 5:15 pm

Re: Topological Naming, My Take

Post by adrianinsaval »

realthunder wrote: Thu Oct 06, 2022 1:34 pm Missing this post some how. Too busy recently. Anyway, the problem you discovered is due a bug introduced by a recent change in Jun. The problem is not really about topo naming. Because when a feature is deleted, the core behavior (both my branch and upstream master) is to break all link to this feature. So the sketch is effectively detached. The bug is caused by the AttachExtension reseting the placement before calculating the new position. The fix is simple, just restore the placement on abort or error. See here.
while restoring the old placement on error brings linkdaily back in parity with master branch it would be nice if the attachment itself used the toponaming algorithm, from my example using fillet/chamfer on the face it is clear that the algorithm is capable of finding the correct face after it breaks so it would seem to be just missing some implementation to make use of the algorithm for attachments. Sketching on faces is one of the big features users seem to want so it's a shame that it doesn't use the toponaming algorithm.
User avatar
chennes
Veteran
Posts: 3878
Joined: Fri Dec 23, 2016 3:38 pm
Location: Norman, OK, USA
Contact:

Re: Topological Naming, My Take

Post by chennes »

@realthunder -- in ComplexGeoData, why did you decide to use QVector for ElementIDRefs? (I ask just because it's the lone Qt inclusion in that class).
Chris Hennes
Pioneer Library System
GitHub profile, LinkedIn profile, chrishennes.com
realthunder
Veteran
Posts: 2190
Joined: Tue Jan 03, 2017 10:55 am

Re: Topological Naming, My Take

Post by realthunder »

chennes wrote: Fri Oct 07, 2022 5:07 pm @realthunder -- in ComplexGeoData, why did you decide to use QVector for ElementIDRefs? (I ask just because it's the lone Qt inclusion in that class).
For the implicit sharing feature, same reason I use QByteArray in StringID and stuff.
Try Assembly3 with my custom build of FreeCAD at here.
And if you'd like to show your support, you can donate through patreon, liberapay, or paypal
realthunder
Veteran
Posts: 2190
Joined: Tue Jan 03, 2017 10:55 am

Re: Topological Naming, My Take

Post by realthunder »

adrianinsaval wrote: Thu Oct 06, 2022 5:05 pm while restoring the old placement on error brings linkdaily back in parity with master branch it would be nice if the attachment itself used the toponaming algorithm, from my example using fillet/chamfer on the face it is clear that the algorithm is capable of finding the correct face after it breaks so it would seem to be just missing some implementation to make use of the algorithm for attachments. Sketching on faces is one of the big features users seem to want so it's a shame that it doesn't use the toponaming algorithm.
I actually implement auto fix in AttachExtension, but not when the linked object gets deleted. Haven't thought about how to handle that yet, as it is a low level action in the core. In my branch, PartDesign feature offers an option to be 'suppressed', and this can be considered as 'feature deletion' without affecting the object linking. Now that you've mentioned it, I did some test. And it seems that the aforementioned attacher changes I added a few month ago broke this auto fix function too. I just fixed it, and you can see the auto fix in action in the screencast below.
attacher.gif
attacher.gif (505.86 KiB) Viewed 1776 times
Try Assembly3 with my custom build of FreeCAD at here.
And if you'd like to show your support, you can donate through patreon, liberapay, or paypal
wsteffe
Posts: 461
Joined: Thu Aug 21, 2014 8:17 pm

Re: Topological Naming, My Take

Post by wsteffe »

From the screenshot I may see that face index is equal to 5 when fillet is activated and is equal to 6 when it is suppressed.
But the Opencascade Class "BRepFilletAPI_MakeFillet" provides the following methodes:

Code: Select all

virtual const TopTools_ListOfShape & 	Generated (const TopoDS_Shape &EorV) override
 	Returns the list of shapes generated from the shape <EorV>. More...
 
virtual const TopTools_ListOfShape & 	Modified (const TopoDS_Shape &F) override
 	Returns the list of shapes modified from the shape <F>. More...
 
virtual Standard_Boolean 	IsDeleted (const TopoDS_Shape &F) override
 	Returns true if the shape S has been deleted.
Using them it should be very easy to associate each modified face with a preexisting face and reuse its original index.
New indices (or new names) should be introduced only for the new (curved) faces generated by the fillet.
The top face of box in the screenshot is clearly a modified (not a generated) face and should keep the index number 6 after making a fillet.
In that way the sketch attachment would be never invalidated after making or removing a fillet.
realthunder
Veteran
Posts: 2190
Joined: Tue Jan 03, 2017 10:55 am

Re: Topological Naming, My Take

Post by realthunder »

wsteffe wrote: Sat Oct 08, 2022 12:50 pm From the screenshot I may see that face index is equal to 5 when fillet is activated and is equal to 6 when it is suppressed.
But the Opencascade Class "BRepFilletAPI_MakeFillet" provides the following methodes:

Code: Select all

virtual const TopTools_ListOfShape & 	Generated (const TopoDS_Shape &EorV) override
 	Returns the list of shapes generated from the shape <EorV>. More...
 
virtual const TopTools_ListOfShape & 	Modified (const TopoDS_Shape &F) override
 	Returns the list of shapes modified from the shape <F>. More...
 
virtual Standard_Boolean 	IsDeleted (const TopoDS_Shape &F) override
 	Returns true if the shape S has been deleted.
Using them it should be very easy to associate each modified face with a preexisting face and reuse its original index.
New indices (or new names) should be introduced only for the new (curved) faces generated by the fillet.
The top face of box in the screenshot is clearly a modified (not a generated) face and should keep the index number 6 after making a fillet.
In that way the sketch attachment would be never invalidated after making or removing a fillet.
The first two functions are the core APIs my algorithm relied on to encode the topo naming. What happens underneath is too complex for your suggestions to work. For example, the sketch shouldn't be aware of the nature of the shape it is attached to, or what kind of changes the shape is going through. In most cases, there are multiple modeling steps inside each PartDesign feature, some steps, i.e. calls to OCC classes provide the above APIs, some do not. And you are required to retain the old shape in order to do the tracing using those APIs. Not just the final result shape, but shapes from all intermediate steps. What my algorithm does is to abstract all these into pure text encoding, and allow trace back and forth without requiring the actual OCC shape.
Try Assembly3 with my custom build of FreeCAD at here.
And if you'd like to show your support, you can donate through patreon, liberapay, or paypal
wsteffe
Posts: 461
Joined: Thu Aug 21, 2014 8:17 pm

Re: Topological Naming, My Take

Post by wsteffe »

realthunder wrote: Sat Oct 08, 2022 11:28 pm you are required to retain the old shape in order to do the tracing using those APIs. Not just the final result shape
But only until you have generated the names of modified faces. Then you may delete the old shape. I Mean something like this:

Code: Select all

  BRepFilletAPI_MakeFillet mkFillet(baseShape);
  const TopoDS_Edge edge = ...;
  mkFillet.Add(radius1, radius2, edge);
  TopoDS_Shape newShape = mkFillet.Shape();

  for (TopExp_Explorer exp(baseShape,TopAbs_FACE); exp.More(); exp.Next()){
     TopoDS_Shape F = exp.Current();
     const TopTools_ListOfShape& RSlist =mkFillet.Modified(F);
     TopoDS_Shape modifiedF=RSit.Value();  //RSList may hold no more than 1 face (0 if original face was unchanged)
     if(modifiedF) indexAndName[modifiedF]=indexAndName[F];
  }
  //Here baseShape is not needed anymore
Of course the general TN algorithm should be suspended (not used) for the face names handled by this code snippet.
Post Reply