Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Problem] User Models break when topological entities change name (Topological Naming Issue) #8432

Open
10 of 15 tasks
sliptonic opened this issue Feb 11, 2023 · 54 comments
Open
10 of 15 tasks
Labels
Core Issue or PR touches core sections (App, Gui, Base) of FreeCAD Known issue Toponaming

Comments

@sliptonic
Copy link
Member

sliptonic commented Feb 11, 2023

Problem description

The FreeCAD Topological Naming issue is well known. It has been a concern for years but no issue exists for it.
The resolution of this issue will involve many individual Pull Requests and probably create numerous secondary issues.

This issue is being created to centralize information, coordinate discussion, and track related topics.

Realthunder has demonstrated a viable solution to the problem in his Link branch. Merging this into master has proven difficult for a variety of reasons.

Anything else?

  • This video gives a reasonable layman's introduction to the problem.
  • This paper gives a more scholarly description of topological naming in general

Realthunder has described how his algorithm works

Related Issues

There's an associated project which gives a nice kanban style view of the progress

Implementation Plan

Phase 1

Phase 2

Other Communication Channels

There's a telegram group for realtime chat about toponaming issues.

Code of Conduct

  • I agree to follow this project's Code of Conduct
@sliptonic sliptonic added Core Issue or PR touches core sections (App, Gui, Base) of FreeCAD Known issue Toponaming labels Feb 11, 2023
@chennes chennes changed the title [Problem] User Models break when topological entities change name (Topologocial Naming Issue) [Problem] User Models break when topological entities change name (Topological Naming Issue) Feb 11, 2023
@luzpaz
Copy link
Contributor

luzpaz commented Feb 11, 2023

JFTR, original Toponaming issue was #6317 which was #922 on Mantis by Jurgen in 2012.

Edit: Ive cleaned up the bbcode and added a link to the FC wiki Toponaming page in sliptonic's OP
Edit2: further refinements to OP

@JohnAD
Copy link
Contributor

JohnAD commented Feb 22, 2023

An almost on-topic question: any recommendations for a good way to do breakpoint-style debugging the C++ code in FreeCAD. I figured I'd ask before I started diving in head-first. Up to this point I've just been analyzing code and mapping out connections.

My normal flow for cpp work is JetBrains CLion for my editor and a newish 16-core laptop running PopOS! (an Ubuntu variant). But I've got lots of tools available including Visual Studio and a 4-core Win10 laptop; and I am happy to switch if there is an advantage.

@berniev
Copy link
Contributor

berniev commented Feb 23, 2023

any recommendations for a good way to do breakpoint-style debugging the C++ code

I'm concerned.

As someone who has been working with C++ for less than a year I was hoping to learn something from the methodology of the paid professional. I was surprised then to find him needing help to do something as trivial as setting a breakpoint. I looked at his YouTube channel, and nothing there is C++ related, and his only C++ github repository is FreeCAD.

@sliptonic
Copy link
Member Author

any recommendations for a good way to do breakpoint-style debugging the C++ code

I'm concerned.

You just questioned the credentials of a new contributor to the project by misquoting him. You left off the in FreeCAD part of his question. This is a person you've never met and your first comment is meant to undermine him. You should be ashamed. The Github issue discussions should have a higher standard of professional discourse and courtesy. In the future, please try to keep your comments constructive.

@chennes
Copy link
Member

chennes commented Feb 23, 2023

Just so it's recorded someplace: the trick to getting CLion to work with FreeCAD is you need to explicitly "Build All" -- for some reason it doesn't get the "ALL_BUILD" target that you would have in Visual Studio.

@berniev
Copy link
Contributor

berniev commented Feb 24, 2023

I too use CLion and there was no trick when I did this a couple of weeks ago. Just do debug build and away you go. Breakpoints work immediately.

@JohnAD
Copy link
Contributor

JohnAD commented Feb 27, 2023

You all have a preferred place for a Gantt chart? If not, I'll throw something into a public repo using https://www.diagrams.net/ .

I should have a crude starting point up this coming week.

(I actually have done one, but I already see holes and errors. I'm going to dive into some more code first.)

@jbaehr
Copy link
Contributor

jbaehr commented Feb 27, 2023

You all have a preferred place for a Gantt chart?

I don't know whether it fits your needs, but you can embedded Gantt charts directly in markdown files on github via mermaid, see https://github.com/mermaidjs/mermaid-gitbook/blob/master/content/gantt.md
This way you have all the benefits you love git for source code for your gantt charts as well.

As a lokal playground, https://kroki.io works great.

@JohnAD
Copy link
Contributor

JohnAD commented Mar 1, 2023

@realthunder

Does the following table and interpretation from the code look correct to you:

postfix (elementMap key) meaning
:H tag (local)
:C child element
:X external tag
:I index
D duplicate
:T deprecated tag (uses decimal)
:M mod
:MG modgen
:G gen
:U upper
:L lower

(above edited based on RealThunder's feedback)

And anything else is most likely an operation.

(for ref: look through code in ComplexGeoData.cpp, TopoShapeEx.cpp, and src/Mod/Part/App/TopoShapeOpCode.h)

And so, the interpretation of the simple Unnamed#Fusion.;Face6;:M2;FUS;:H1ab:8,F.Face10 (asuming Hashing turned off) would be

Unnamed = the documents name

# = doc / object separator

Fusion = object name

. = object / elementMap name separator

MappedName portion begins

; = elementMap entry prefix

Face6 = the item's historic parent element name

; = elementMap entry prefix

:M = marker of modification
2 = the object at index 2 (the third object)

; = elementMap entry prefix

FUS = modification was a fusion operation; see src/Mod/Part/App/TopoShapeOpCode.h

; = elementMap entry prefix

:H = This is a TAG
1ab = the tag's id in hex (would be '-' if it didn't have one)
:8 = marker indicating length of _previous_prefix, in decimal
, = comma separator for next parameter for ':H'
F = This is a "Face"; literally the first letter of the actual name

MappedName portion stops

. = MappedName / element's name separator

Face10 = the actual name for this element

@JohnAD
Copy link
Contributor

JohnAD commented Mar 1, 2023

@berniev

I apologize for not responding sooner. I am a generalist. While it is true that I've been using C++ since version 2.0 in the late 90s; that isn't enough. I've learned that most large systems; especially ones written in non-idiomatic languages like C++, C, and assembler, are never in a predictable pattern. Each system has a team that has learned behaviors, formatting, tools, and institutional knowledge. So, I'm happy to ask basic questions like "what is the recommended way of doing such and such". Not just because it can help (it can), but the questions usually bring in more insight.

Any more, I also like to pair program for a bit with a new client, but I'm not sure I can pull that off with an open source group :-)

As to the need to do ALL_BUILD at least once. On my machine, it seems to be a requirement as CLion is not detecting that a lot of the supporting elements have not been generated/moved/etc (such as SVG files, etc.) until it is run at least once. After that, I can simply use the 'Debug FreeCadMain` icon on the toolbar and away it goes.

@realthunder
Copy link
Collaborator

@JohnAD In my code, I call those elementMap key that you listed as postfix, which includes the elementMapPrefix(), i.e. ;.

Correction: the modgen postfix is ;:MG, because in the code, it is defined as modPostfix() + "G".

;:T is an deprecated version of the tag postfix, serving the same purpose of ;:H. dec in the code means decimal because it encodes object ID using decimal digits instead of hex. The postfix is only used in one of my old branch. It is included for backward compatibility.

Just some quick notes. I'll add more later.

@Zolko-123
Copy link
Contributor

Wouldn't the first step of investigating this TopoNaming issue be to trace the usage and whereabouts of the 2 functions setElementName() and getElementName() in realthunder's branch ?

https://github.com/realthunder/FreeCAD/blob/LinkDaily/src/App/ComplexGeoData.cpp#L1587
https://github.com/realthunder/FreeCAD/blob/LinkDaily/src/App/ComplexGeoData.cpp#L1456

at least that's where I would begin the journey

@JohnAD
Copy link
Contributor

JohnAD commented Mar 2, 2023

Breaking this into to planning parts. First, the general pattern:

flowchart TD
    id1([Prepare])
    id2([Implement])
    id3([Integrate])
    id4>Enable]
    id5(Optimize)
    id1 --> id2
    id2 --> id3
    id3 --> id4
    id4 --> id5
    id5 --> id5
  1. (Prepare)
    Create the supporting functions needed by element naming and the algorithm.
    These are simply libraries with good unit testing to ensure they are accurate when used later.
    FreeCAD behavior is unchanged for users
  2. (Implement)
    Create the naming-of-elements algorithm.
    Create the ElementMap class.
    Using the previous libraries, import the general algorithm that will eventually be called by ComplexGeoData.cpp to have the more advanced naming.
    Add the members to the ComplexGeoData.cpp class but don't use them. Simply add support for visibility in the Selection View and support for the manual call via Python Console.
    Using unit tests, verify that the naming is very predictable and reliable.
    FreeCAD behavior is unchanged for users
  3. (Integrate)
    Migrate to v.22; contact other workbench maintainers.
    Add the naming, but do not use it yet.
    While the member values are set, they are not actually used.
    This stage will slow down FreeCAD and will make the naming visible on an element when selected.
  4. (Enable)
    Use the toponaming so that the model is more resilient to upstream object changes.
    Users will start seeing increased stability. Performance is still impacted
  5. (Optimize)
    Selectively add optimizations to speed FreeCAD up again.
    This could include: MappedName-hashing and IndexedName-int:string-caching.
    Repeat

@JohnAD
Copy link
Contributor

JohnAD commented Mar 2, 2023

Only 13 individual steps so far, but I'm sure there will be more as we discuss things. Any and all insight/feedback is great as you all have much experience with the structure of FreeCAD.

The markdown file references below are from a scratch repo: https://github.com/JohnAD/temp_toponaming_gannt_chart

Much of this is mild reorganization since the basics are already written by RealThunder. But adding test cases and slower migration will help with organizing this stuff in our collective heads.

  1. (Prepare)

    Are there any other clean libraries we could write to help with the following steps?

  2. (Implement)

  3. (Integrate)

    • Adjustments to Sketcher workbench

    • Adjustments to Part workbench

    • Adjustments to ComplexGeoDataPy

  4. (Enable)

    • Adjustments to Save / Restore processes

    • Adjustments to Sketcher workbench

    • Adjustments to Part workbench

  5. (Optimize)

    • consider MappedName hashing

    • consider IndexedName int:string map cache per-project or system-wide.

@JohnAD
Copy link
Contributor

JohnAD commented Mar 2, 2023

The steps are not in a gantt chart yet as I'm still untangling order in my head for steps 3 and 4; but I'm not far.

@sliptonic
Copy link
Member Author

sliptonic commented Mar 2, 2023

Here's the start of a Gantt chart.

  • Start date was arbitrarily set to 3/15 just to work on the chart.
  • All task dates are currently set to 10d because better estimates aren't available yet.
  • Task concurrency is strictly by phase now. That will likely change.
  • This chart will undergo heavy editing in the future.

In other words, assuming anything about 'when' by reading this chart is foolish. It's guaranteed to be wrong.

gantt
    title Toponaming Fix Plan
    dateFormat  YYYY-MM-DD

    section Prepare
    start            : milestone, m1, 2023-03-15,2min
    IndexedName      :p1, after m1, 10d
    MappedName       :p2, after m1  , 10d
    ElementMap.h     :p3, after m1  , 10d
    end            : milestone, m2, after p3,2min

    section Implement
    ComplexGeoData.cpp classes :i1, after m2, 10d
    ComplexGeoData.cpp/h selection view visibility :i2, after i1, 10d
    ElementMap.cpp   :i3, after i2  , 10d
    user-defined element naming :i4, after i3, 10d

    end           : milestone, m3, after i4, 2min

    Release V0.21: milestone, R1, after m3, 2min

    section Integrate
    Adjustments to Sketcher workbench   :ii3, after R1  , 10d
    Adjustments to Part workbench       :ii2, after R1  , 10d
    Adjustments to ComplexGeoDataPy     :ii4, after R1  , 10d
    end            : milestone, m4, after ii4 ,2min

    section Enable
    Adjustments to Save / Restore processes    :e1, after ii4  , 10d
    Adjustments to Sketcher workbench          :e3, after ii4  , 10d
    Adjustments to Part workbench              :e2, after ii4  , 10d
    end            : milestone, m5, after e3, 2min

    section Optimize
    MappedName hashing?                                             :o1, after e3  , 10d
    IndexedName? int:string map cache per-project or system-wide.   :o2, after e3  , 10d
    end            : milestone, m6, after o2, 2min

@adrianinsaval
Copy link
Member

adrianinsaval commented Mar 3, 2023

  • Adjustments to Part workbench
  • Adjustments to Sketcher workbench

Considering that sketcher is the one that will get the strongest stability and already provides a lot of functionality outside of it, and in light of many previous comments from zolko, is it possible to do sketcher first and then Part? Or is the Part side an unavoidable prerequisite for doing it in sketcher?

@adrianinsaval
Copy link
Member

3. Add support for visibility in the Selection View.
This stage will slow down FreeCAD and will make the naming visible on an element when selected.

This is very nice and desired (and I think not currently provided by rt's solution), but the mapped names seem very long and not too user friendly, is this what will be displayed? Can we find something else? Like unique numbers or something.

@JohnAD
Copy link
Contributor

JohnAD commented Mar 3, 2023

  1. Add support for visibility in the Selection View.
    This stage will slow down FreeCAD and will make the naming visible on an element when selected.

This is very nice and desired (and I think not currently provided by rt's solution), but the mapped names seem very
long and not too user friendly, is this what will be displayed? Can we find something else? Like unique numbers or
something.

The long mapped names would only be seen in the Selection View window. Elsewhere only the short name is seen. And, certainly, we can even change that behavior later during the optimization phase. There is no real need for the end user to see the mapped names; it is more of a developer convenience.

Perhaps during the optimization dev phase, we could have a global flag settable in the Python Console that lets developers see the mapped name, otherwise the simpler name is shown?

@JohnAD
Copy link
Contributor

JohnAD commented Mar 3, 2023

I've made edits to the charts above based on changes made in a meeting with other devs this morning.

@JohnAD
Copy link
Contributor

JohnAD commented Mar 7, 2023

I've added the first 4 cards as Issues and edited the above chart. I'll start generating Phase 2 cards next. Those get somewhat more complicated but shouldn't be too bad.

@pinpox
Copy link

pinpox commented Mar 7, 2023

I'm subscribed to this issue because I stumbled across the topo naming problem a few times and am excited for upcoming fixes for it. I read through this issue, but have trouble understanding if it will be fixed by this and what the fix will actually be.

Is the problem, current state and solution documented somewhere? As a newish user of freecad it would be helpful to know how likely this will solve the problem from a users perpective and what the connection to @realthunder 's branch is. Should I be using that? Should I just wait for the next official release?

I assume I'm not the only one stumbling over this issue, maybe someone can link an issue to read up on the plans

@JohnAD
Copy link
Contributor

JohnAD commented Aug 13, 2023

Proposed cards for Phase 3:

Phase 3: Integrate

Add the naming, but do not use it yet. So, while the member values are set, they are not actually used. This stage will slow down FreeCAD and will make the naming visible on an element when selected.

You will see some items marked with a question mark (?). These are where I'm not sure the changes are TN related and might be skipped or done later.

  • Add History Support Naming is based on history. Add the RT changes to history to expand it's support.
    • Part Workbench
      • PropertyTopoShape (do before the next set of cards)
      • card 1.4: Write unit tests covering Toposhape current behavior.
      • card 1.5: Add the 60+ new methods and private "wrapper" class to TopoShape. (See TopoShape Ex.)
      • PartFeature (just the History parts like 'buildHistory' and 'joinHistory')
      • FeaturePartCommon
      • FeaturePartExtension?
      • FeatureChamfer
      • FeatureFillet
      • FeaturePartBoolean
      • FeaturePartFuse
      • FeatureExtrusion
      • FeatureRevolution
    • Part Design Workbench
      • ShapeBinder
      • FeatureDressUp (just internal identifier naming?)
      • FeatureSketchBased
      • FeatureTransformed (just internal identifier naming?)
    • Sketcher Workbench
      • SketchObject (see GeoHistory and updateGeoHistory)
  • Add Name Assignment Based on the advanced history, generate the names and assign to Doc Objects
    • Part Workbench. This includes saving and restoring.

      not all of these can be worked independently, lessons will be learned 😄

      • TopoShape and TopoShapeEx (BIG card)
      • PartFeature
      • Attacher
      • AttachExtension
      • Geometry and GeometryExtension and GeometryMigrationExtension?
      • CrossSection?
      • FaceMaker
      • FeatureExtrusion
    • Part Design Workbench
      • ShapeBinder
    • Sketcher Workbench
      • SketchObject ('buildShape' change and related) (what is toggleFreeze used for?)
      • ExternalGeometryExtension, ExternalGeometryFacade, GeometryFacade (set/get for saving/restore)
      • SketchGeometryExtension (set/get for saving/restore)
  • Add Python Support Add in the Py and PyImp changes for viewing the new names
    • Part Workbench
      • Command
      • AppPartPy
      • AttachEnginePyImp?
      • PartFeaturePy and PartFeaturePyImp
      • TopoShape * Py *
    • Part Design Workbench
      • Command
    • Sketcher Workbench
      • CommandAlterGeometry, CommandCreateGeo, CommandSketcherTools
      • SketchObjectPy, SketchObjectPyImp

Phase 3

  1. [Issue] TopoNaming Phase 3 (card 1 of 35) - Add History Support/Part Workbench/PropertyTopoShape #10353
  2. [Issue] TopoNaming Phase 3 (card 2 of 35) - Add History Support/Part Workbench/PartFeature #10354
  3. [Issue] TopoNaming Phase 3 (card 3 of 35) - Add History Support/Part Workbench/FeaturePartCommon #10355
  4. [Issue] TopoNaming Phase 3 (card 4 of 35) - Add History Support/Part Workbench/FeaturePartExtrusion #10356
  5. [Issue] TopoNaming Phase 3 (card 5 of 35) - Add History Support/Part Workbench/FeatureChamfer #10357
  6. [Issue] TopoNaming Phase 3 (card 6 of 35) - Add History Support/Part Workbench/FeatureFillet #10358
  7. [Issue] TopoNaming Phase 3 (card 7 of 35) - Add History Support/Part Workbench/FeaturePartBoolean #10359
  8. [Issue] TopoNaming Phase 3 (card 8 of 35) - Add History Support/Part Workbench/FeaturePartFuse #10360
  9. [Issue] TopoNaming Phase 3 (card 9 of 35) - Add History Support/Part Workbench/FeatureExtrusion #10361
  10. [Issue] TopoNaming Phase 3 (card 10 of 35) - Add History Support/Part Workbench/FeatureRevolution #10362
  11. [Issue] TopoNaming Phase 3 (card 11 of 35) - Add History Support/Part Design Workbench/ShapeBinder #10398
  12. [Issue] TopoNaming Phase 3 (card 12 of 35) - Add History Support/Part Design Workbench/FeatureDressUp #10399
  13. [Issue] TopoNaming Phase 3 (card 13 of 35) - Add History Support/Part Design Workbench/FeatureSketchBased #10400
  14. [Issue] TopoNaming Phase 3 (card 14 of 35) - Add History Support/Part Design Workbench/FeatureTransformed #10401
  15. [Issue] TopoNaming Phase 3 (card 15 of 35) - Add History Support/Sketcher Workbench/SketchObject #10402

@JohnAD
Copy link
Contributor

JohnAD commented Aug 13, 2023

Please make comments and suggestions. I know more about the Part workbench than the Sketcher workbench; so all Sketcher help is extra appreciated 😄 .

@JohnAD
Copy link
Contributor

JohnAD commented Aug 24, 2023

Over the course of the next 24 hours, I will be creating 32-or-so Issues named in the form of:

[Issue] TopoNaming Phase 3 (card 1 of 32) - Add History Support/Part Workbench/PropertyTopoShape

They will be linked into the opening comment/description at the top of this issue; and they will be linked to the TopoNaming project.

This was referenced Aug 25, 2023
@JohnAD
Copy link
Contributor

JohnAD commented Oct 23, 2023

After starting up the cards, it appears that the "TopoShape and TopoShapeEx (BIG card)" mentioned in Part 2 the list above must be done before the remaining parts of Part 1. So, I'm adding moving that card after card 1 and before card 2. Essentially:

  • card 1.4: Write unit tests covering Toposhape current behavior.
  • card 1.5: Add the 60+ new methods and private "wrapper" class to TopoShape. (See TopoShape Ex.)

@AlexDaniel
Copy link

Any updates? Phase 3 was planned some time ago, but the work wasn't started.

@chennes
Copy link
Member

chennes commented Dec 12, 2023

We are working on the TopoShape class -- it is massive (94 added methods in 10,000 lines of code). Right now we are waiting for some additional documentation about a caching mechanism from @realthunder.

@Timothy-Ecc19
Copy link

Great to hear this is still being worked on!

@furgo16
Copy link
Contributor

furgo16 commented Feb 19, 2024

Many thanks for the work everyone is doing towards solving this issue. For those of us who are following the progress, would it be possible for a maintainer to add the "Phase 3" issue list (in #8432 (comment) or by the list of issue references to the main issue description?

This is just a suggestion to make it easier to potential contributors to see what's being done and one left from a SSoT (Single Source of Truth) position without having to navigate through all the comments.

If it helps, here's the markdown to copy and paste the current list of issues

#### Phase 3

1. https://github.com/FreeCAD/FreeCAD/issues/10353
1. https://github.com/FreeCAD/FreeCAD/issues/10354
1. https://github.com/FreeCAD/FreeCAD/issues/10355
1. https://github.com/FreeCAD/FreeCAD/issues/10356
1. https://github.com/FreeCAD/FreeCAD/issues/10357
1. https://github.com/FreeCAD/FreeCAD/issues/10358
1. https://github.com/FreeCAD/FreeCAD/issues/10359
1. https://github.com/FreeCAD/FreeCAD/issues/10360
1. https://github.com/FreeCAD/FreeCAD/issues/10361
1. https://github.com/FreeCAD/FreeCAD/issues/10362
1. https://github.com/FreeCAD/FreeCAD/issues/10398
1. https://github.com/FreeCAD/FreeCAD/issues/10399
1. https://github.com/FreeCAD/FreeCAD/issues/10400
1. https://github.com/FreeCAD/FreeCAD/issues/10401
1. https://github.com/FreeCAD/FreeCAD/issues/10402

Thanks!

@Pesc0
Copy link
Contributor

Pesc0 commented Feb 19, 2024

this should be what you're looking for https://github.com/orgs/FreeCAD/projects/2, let me know if that helps

@furgo16
Copy link
Contributor

furgo16 commented Feb 19, 2024

this should be what you're looking for https://github.com/orgs/FreeCAD/projects/2, let me know if that helps

I'm aware of the GitHub project, which is also mentioned in the issue description. But thanks for pointing it out in any case!

@AlexDaniel
Copy link

Wow, it looks like there has been a lot of progress lately! Keep it up! ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core Issue or PR touches core sections (App, Gui, Base) of FreeCAD Known issue Toponaming
Projects
Status: Backlog
Development

No branches or pull requests