I had the idea of doing a "simple but imperfect" implementation of topological naming. The idea being that it can be reviewed and tested a lot faster so we can handle at least most of the "jumping sketch" and "lost edge reference for fillet" problems as soon as possible.
Its here:
https://github.com/jrheinlaender/FreeCA ... naminglite
The goal was to keep the code as simple as possible and the changes to PartDesign Feature code as small as possible. The trade-off being that not all situations are handled. I estimate that 50-80% of the "jumping sketch" etc. problems will be caught by this code.
Let me know what you think. The place to start looking at the code is in Part/App/PartFeature.h
[Open] Topological Naming code
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
Be nice to others! Respect the FreeCAD code of conduct!
-
- Posts: 554
- Joined: Sat Apr 07, 2012 2:42 am
Re: Topological Naming code
Very interesting! Is this already tested for sketches to fix this jumping problem? Do you have examples where it fails? Then what is the reason to start another naming implementation? AFAIK you've already implemented the naming stuff in a separate branch. Are there any attempts to merge both or what are your plans?
Programming stuff:
This produces some dangling pointers because the keys are still there. And in the case a key is not present in the map you create it this way.
Programming stuff:
Code: Select all
/// Clear the data structure created by extractSubShapes()
void clearSubShapes(std::map<TopAbs_ShapeEnum, TopTools_IndexedMapOfShape*> map)
{
for (int t = 0; t < numShapeTypes; t++)
delete map[shapeTypes[t]];
}
Code: Select all
/// Clear the data structure created by extractSubShapes()
void clearSubShapes(std::map<TopAbs_ShapeEnum, TopTools_IndexedMapOfShape*> map)
{
for (int t = 0; t < numShapeTypes; t++) {
std::map<TopAbs_ShapeEnum, TopTools_IndexedMapOfShape*> map:: iterator it;
it = map.find(shapeTypes[t]);
if (it != map.end()) {
delete it->second;
it->second = 0;
}
}
}
-
- Posts: 554
- Joined: Sat Apr 07, 2012 2:42 am
Re: Topological Naming code
The other branch is highly complex and will therefore take a long time to find its way into master (its already about a year old!!) Also, it only handles the history building at this moment. How to actually adjust properties like the sketch face using the history is still totally unclear (according to jriegel).Then what is the reason to start another naming implementation? AFAIK you've already implemented the naming stuff in a separate branch.
So the idea was to have a simple implementation that handles most cases and which (hopefully) can be merged quickly.
No, because two totally different methods are being used. Exchanging one naming implementation for another later on should be no problem as long work correctly, since naming should happen in the background, "unseen" by the user, anyway.Are there any attempts to merge both or what are your plans?
As I said, it works "most of the time". In the code I have documented the cases that are not covered (e.g. the end shape of BRepFeat_MakePrism). There are plenty of cases where it fails, but in that case the code simply doesn't alter anything so we are no worse off than before.Is this already tested for sketches to fix this jumping problem? Do you have examples where it fails?
So, my reasons for doing this are:
1. Some naming is better than no naming at all
2. The changes to the Feature::execute() methods are minimal
3. For the cases when this method works, the user will be happy. For the cases that are not handled the user won't be any more upset than he is already
4. It was simple and quick to implement
Thanks for pointing out the error in the code!
-
- Posts: 554
- Joined: Sat Apr 07, 2012 2:42 am
Re: Topological Naming code
I thought about this again. The dangling pointers don't worry me because the map will run out of scope immediately afterwards everywhere that it is cleared in this way. And I am sure all the keys are in the map, otherwise there would have been a crash much earlier on. So isn't this the simplest way of freeing the pointer memory?This produces some dangling pointers because the keys are still there. And in the case a key is not present in the map you create it this way.
Re: Topological Naming code
This is true as long as the function is used exactly this way. But what if someone else later uses this function and doesn't know about this? After all the function causes undefined behaviour in certain situations and this is something that should be avoided. Furthermore, the fixed version is not that much slower so that it does no harm.