Topo Naming PR 1 #7427
Merged
Topo Naming PR 1 #7427
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
By replacing dynamic allocated QAtomicInt with std::atomic and making most code inlined.
This commit is split out from realthunder/FreeCAD#a2579bff, which introduced support of saving FreeCAD document as directory. This commit only include relavent changes for efficient storage of topological naming. Full support of directory saving will be added in another PR. The specific changes needed for topological naming storge is the proper support of XML characters content in Base::XMLReader and Base::Writer. Use XMLReader::beginCharStream() to stream either binary or text data. Binary data will be base64 encoded on the fly with user defined line size. The base64 string is stored as plain character content inside the current XML element. Text data is stored inside CDATA section with automatic detection and handling of CDATA end tag inside the text. Be WARNED though, there is no validation of text character yet. The caller must ensure the input text is valid UTF8. Topological naming uses text stream. The change introduced here also affects App::PropertyFileIncluded, Gui::TreeWidget (for persistence of tree item expansion status), Path::Toolpath, Points::PointKernel, and Spreadsheet::Cell. The changes by right shouldn't cause any backward compatibility issue. Another relatively small change that is not so relavant to topo naming is the support of hierarchical XMLReader, which allows the top level reader to centralize control of mult-file-reading that might be added by lower level xml reader. This change is included here because of difficulty in splitting the functionality apart from the streaming changes. Note that this change breaks Mod/Cloud module, which will be fixed in follow up patches.
General purpose string table to avoid saving duplicated text string multiple times.. Mainly used by topological naming to compress model histroy. But can be used for other purpose as well, e.g. to strore documentation of dynamic property.
in App::Property and App::PropertyContainer, implemented in App::Document/DocumentObject, and Gui::ViewProviderDocumentObject
Added in class Property and class PropertyContainer to allow customized processing before saving.
Also add DocumentObject::signalEarlyChanged, triggered before Document::signalChangedObject
These are the fundation for storing and querying the new topological naming of geometry element. To get an overview of this fundation, please check out the following article https://github.com/realthunder/FreeCAD_assembly3/wiki/Topological-Naming There are some changes in topo naming internal storage, which changes from plain string to two dedicated class IndexedName (for indexed geometry element name) and MappedName. These two classes are created to improve topological naming performance in terms of speed, runtime memory, and persistence storage size.
For comparing if two geometries are the same. Exposed to Python with a method of the same name. The API will be used by TopoShape to search shape by geometry to make topological naming more robust.
The topological naming aware member functions are all located in a new source file TopoShapeEx.cpp. The core algorithm of generate the names is in TopoShape::makESHAPE(). Check out the following wiki article for more details. https://github.com/realthunder/FreeCAD_assembly3/wiki/Topological-Naming-Algorithm Note that the actual name encoding is slightly changed to improve efficiency, but the algorithm is mostly the same.
Only save to XML if asked explicitly
Only save to XML if asked explicitly
Same for TopoShapePy.getElement()
github-actions
bot
added
Core
|
Snap builds of toponaming branch are being built nightly at https://github.com/FreeCAD/FreeCAD-snap/actions |
realthunder
force-pushed
the
development/toponaming
branch
from
5b4b539
to
531a94a
Compare
|
@luzpaz Why does speller now check into source code? It even complains about variable name miss spell. |
Closed
Comment on lines
1220
to
1230
| { | ||
| int index = 0; | ||
| std::string element; | ||
| boost::regex ex("^([^0-9]*)([0-9]*)$"); | ||
| boost::cmatch what; | ||
|
|
||
| if (boost::regex_match(name, what, ex)) { | ||
| element = what[1].str(); | ||
| index = std::atoi(what[2].str().c_str()); | ||
| } | ||
|
|
||
| return getSubElement(element.c_str(), static_cast<unsigned long>(index)); | ||
| int index = 0; | ||
| std::string element(name); | ||
| std::string::size_type pos = element.find_first_of("0123456789"); | ||
| if (pos != std::string::npos) { | ||
| index = std::atoi(element.substr(pos).c_str()); | ||
| element = element.substr(0,pos); | ||
| } | ||
|
|
||
| return getSubElement(element.c_str(),index); | ||
| } |
There was a problem hiding this comment.
Is it necessary to remove the regular expression here?
Some months ago I added the use of regex in some parts of the code to avoid using wrong names for subelements.
There was a problem hiding this comment.
Yeah, I didn't notice that. The code is copied from older version before your patch. I'll revert it back.
chennes
reviewed
Sep 25, 2022
Comment on lines
+415
to
+435
| /** Maps an arbitrary string to an integer | ||
| * | ||
| * The function maps an arbitrary text string to a unique integer ID, which | ||
| * is returned as a shared pointer to reference count the ID so that it is | ||
| * possible to prune any unused strings. | ||
| * | ||
| * If the string is longer than the threshold setting of this StringHasher, | ||
| * it will be sha1 hashed before storing, and the original content of the | ||
| * string is discarded. | ||
| * | ||
| * The purpose of function is to provide a short form of a stable string | ||
| * identification. | ||
| */ | ||
| StringIDRef getID(const char *text, int len=-1, bool hashable=false); | ||
|
|
||
| /** Map text or binary data to an integer */ | ||
| StringIDRef getID(const QByteArray & data, bool binary, bool hashable=true, bool nocopy=false); | ||
|
|
||
| /** Map geometry element name to an integer */ | ||
| StringIDRef getID(const Data::MappedName & name, | ||
| const QVector<StringIDRef> & sids); |
There was a problem hiding this comment.
@realthunder can you add documentation of the parameters here? Also, @wwmayer, are you concerned about boolean blindness in the first two of these functions? The first one only has the single boolean at the end, but the second has three, which I suspect in client code is pretty opaque.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the first batch of patches for the new Topological Naming, which names the geometry sub-element (Vertex, Edge, Face) using the modeling history. The purpose of the new naming algorithm is to improve robustness of sub-element references used by features in later modeling steps. To maximize source code backward compatibility, the old index based element names (e.g. Vertex3, Face2) are still accepted as sub-element references. The new Topo Naming is used internally to track changes of the element index and auto correct the index based element names in the reference.
The first batch of patches makes necessary core changes to build up the foundation for generation and utilization of the new Topo Naming. There is no visible changes for the end-user at this stage. The patches introduces a few new classes and adds some new APIs to existing classes (mostly in ComplexGeoData, TopoShape, and various Property links with sub names). There are also some changes to Base::Reader and Writer, for more efficient embedding of arbitrary binary and unicode text into XML file, which is required for topo naming persistence. These Reader and Writer changes are also the precursor to allow saving FreeCAD document as an uncompressed directory, which will be introduced in future PR that is unrelated to Topo Naming.
The next batch of patches adds code to the features from Part workbench for generating Topo Naming using the new APIs. At this stage, the end-user can see the new Topo Naming in action by testing various features from Part workbench. Only features from Part workbench are supported. And if sketch is involved (e.g. Extrusion from a sketch), then try not to modify the sketch for the purpose of Topo Naming testing until the next batch of patches is in.
The third batch of patches modifies the Sketcher workbench to fully support the new Topo Naming. At this stage, the end-user can see the Topo Naming in full action for all features in Part workbench.Changes like adding or removing geometries in sketch will either have no effect to the existing model, or fail gracefully and is easy to repair manually.
The forth and final batch of patches is aimed to fully support Topo Naming in PartDesign workbench, making FreeCAD as a whole (more or less) free from the decade-old Topological Naming problem.
For more details about the internals of the algorithm please check the links below
https://github.com/realthunder/FreeCAD_assembly3/wiki/Topological-Naming
https://github.com/realthunder/FreeCAD_assembly3/wiki/Topological-Naming-Algorithm