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!
User avatar
adrianinsaval
Veteran
Posts: 5541
Joined: Thu Apr 05, 2018 5:15 pm

Re: Topological Naming, My Take

Post by adrianinsaval »

Zolko wrote: Fri Jul 15, 2022 3:36 pm can you please point where in the code this is done ? I tried and failed to find the relevant files.
Sorry I'm not very familiar with sketcher's codebase but it is clear that it just a list of objects, when something is deleted it is removed from the list and the following items now have a reduced index, the same thing happens with constraint, note that the actual Shape property of sketches also gets it's toponaming from OCCT. I was referring to these lists here:
Untitled.png
Untitled.png (10.22 KiB) Viewed 2728 times
I like your idea to solve this issue and I believe you have already proposed it before, instead of keeping a copy of the geometry data with a "deleted" property I would prefer empty elements in the lists (with a deleted property to differentiate). I don't know if the geometry index are important at all since in the end it's the OCCT given names that get referenced outside the sketch but the constraint index are important for expressions, it's very noticeable when using carbon copy.
@abdullah how hard would it be to make the geometry or at least the constraint index permanent? If you have a desire to have a python accessible consecutive list to be used for example in loops we could have a separate property that stores the indexes of the non deleted elements. It might be a good idea to discuss these in a separate thread though as it is not exactly the same as the toponaming problem, just similar, it's the sketcher constraint/geometry index problem.
As a top-level note, I think that solving/improving the topological naming is a big goal for v0.21, but we could do this in steps: first do it for Sketches only in v0.21, and THEN do it for other things for v0.22. My fear is that solving the TNP for EVERYTHING in 1 giant step in v0.21 might be a very long road, and having solved it for sketches would solve many MasterSketch / DatumObjects combinations (you might see where this is coming from)
The objective is to merge the realthunder stuff, his code doesn't really cover everything, it covers sketcher (but not these indexes I think?) Part and Part Design. If it takes too long I guess we could do it in parts but IMO if it's possible we should strive to get all 3 merged.
User avatar
ppemawm
Veteran
Posts: 1240
Joined: Fri May 17, 2013 3:54 pm
Location: New York NY USA

Re: Topological Naming, My Take

Post by ppemawm »

Zolko wrote: Fri Jul 15, 2022 3:36 pm As a top-level note, I think that solving/improving the topological naming is a big goal for v0.21, but we could do this in steps: first do it for Sketches only in v0.21
+1
"It is a poor workman who blames his tools..." ;)
logari81
Posts: 658
Joined: Mon Jun 14, 2010 6:00 pm

Re: Topological Naming, My Take

Post by logari81 »

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.
chrisb
Veteran
Posts: 53919
Joined: Tue Mar 17, 2015 9:14 am

Re: Topological Naming, My Take

Post by chrisb »

logari81 wrote: Thu Aug 11, 2022 9:17 am I have read RealThunder's description of his solution and looks absolutely convincing to me.
Convincing is not enough. From what I understood, it has to be integrated into the master codebase in a way that is future safe, so that not only realthunder is able to maintain the code himself.
A Sketcher Lecture with in-depth information is available in English, auf Deutsch, en français, en español.
logari81
Posts: 658
Joined: Mon Jun 14, 2010 6:00 pm

Re: Topological Naming, My Take

Post by logari81 »

Convincing is certainly not enough, that's why I am asking if there is any list with shortcomings/issues.

Regarding maintainability. People are maintaining code in FreeCAD which is much less documented that the one by realthunder. I wish I had documented my sketcher implementations half as good as RealThunder has done for his code. People could nevertheless maintain my code and extend it significantly. So this is not really an issue.

The real question is: are there any use-cases/examples that fail with realthunder's branch?
User avatar
adrianinsaval
Veteran
Posts: 5541
Joined: Thu Apr 05, 2018 5:15 pm

Re: Topological Naming, My Take

Post by adrianinsaval »

logari81 wrote: Thu Aug 11, 2022 11:36 am The real question is: are there any use-cases/examples that fail with realthunder's branch?
what constitutes a fail to you? If you mean something that can break due to toponaming, the existence of these is not really something that should stop the merge of the algorithm, I do not expect it to be perfect (it's impossible to 100% solve TNP) and anything is better than the current state anyways.
The issue is that there's a ton of merge conflicts that needed to be sorted out first and then the code itself has to be acceptable and sufficiently understood by the maintainers.
chrisb
Veteran
Posts: 53919
Joined: Tue Mar 17, 2015 9:14 am

Re: Topological Naming, My Take

Post by chrisb »

I remember a comment from sliptonic who couldn't understand how some code from realthunder for the Path workbench really worked. He finally merged it in the hope that things would work out well. Topological naming is a much bigger and more important task, so it is probably a good idea if much more care is taken.
A Sketcher Lecture with in-depth information is available in English, auf Deutsch, en français, en español.
chrisb
Veteran
Posts: 53919
Joined: Tue Mar 17, 2015 9:14 am

Re: Topological Naming, My Take

Post by chrisb »

Some posts have been moved from this topic to a new one: https://forum.freecadweb.org/viewtopic.php?f=21&t=71180.
A Sketcher Lecture with in-depth information is available in English, auf Deutsch, en français, en español.
abdullah
Veteran
Posts: 4935
Joined: Sun May 04, 2014 3:16 pm
Contact:

Re: Topological Naming, My Take

Post by abdullah »

adrianinsaval wrote: Fri Jul 15, 2022 4:44 pm @abdullah how hard would it be to make the geometry or at least the constraint index permanent? If you have a desire to have a python accessible consecutive list to be used for example in loops we could have a separate property that stores the indexes of the non deleted elements. It might be a good idea to discuss these in a separate thread though as it is not exactly the same as the toponaming problem, just similar, it's the sketcher constraint/geometry index problem.
All code relies on the position in the list being the geoId. It is about rewritting the whole thing. Same applies to constraint index.

RT's branch has another separate index (a global geometry index). In fact, we have this index too, but it is not exposed as it is intended to be used via RT's functionality (it is somehow, reserved for his code until it is merged). It kind of thought he had solved the toponaming issue for sketches (but I must be wrong).
User avatar
adrianinsaval
Veteran
Posts: 5541
Joined: Thu Apr 05, 2018 5:15 pm

Re: Topological Naming, My Take

Post by adrianinsaval »

I think he has, but zolko kinda wanted to start over just for sketches. Later I said that IMO the geometry list is not an issue as I have never seen that directly causing problems, it's the edge index from the Shape property that cause TNP. On the other hand the constraint list is problematic because they can be referred to in expressions (specially when using carbon copy) so it's problematic.
Post Reply