As described in Issue 16849, the existing Tools::getUniqueName method
requires calling code to form a vector of existing names to be avoided.
This leads to poor performance both in the O(n) cost of building such a
vector and also getUniqueName's O(n) algorithm for actually generating
the unique name (where 'n' is the number of pre-existing names).
This has particularly noticeable cost in documents with large numbers
of DocumentObjects because generating both Names and Labels for each new
object incurs this cost. During an operation such as importing this
results in an O(n^2) time spent generating names.
The other major cost is in the saving of the temporary backup file,
which uses name generation for the "files" embedded in the Zip file.
Documents can easily need several such "files" for each object in the
document.
This update includes the following changes to use the newly-added
UniqueNameManager as a replacement for the old Tools::getUniqueName
method and deletes the latter to remove any temptation to use it as
its usage model breeds inefficiency:
Eliminate Tools::getUniqueName, its local functions, and its unit tests.
Make DocumentObject naming use the new UniqueNameManager class.
Make DocumentObject Label naming use the new UniqueNameManager class.
This needs to monitor DocumentObject Labels for changes since this
property is not read-only. The special handling for the Label
property, which includes optionally forcing uniqueness and updating
links in referencing objects, has been mostly moved from
PropertyString to DocumentObject.
Add Document::containsObject(DocumentObject*) for a definitive
test of an object being in a Document. This is needed because
DocumentObjects can be in a sort of limbo (e.g. when they are in the
Undo/Redo lists) where they have a parent linkage to the Document but
should not participate in Label collision checks.
Rename Document.getStandardObjectName to getStandardObjectLabel
to better represent what it does.
Use new UniqueNameManager for Writer internal filenames within the zip
file.
Eliminate unneeded Reader::FileNames collection. The file names
already exist in the FileList collection elements. The only existing
use for the FileNames collection was to determine if there were any
files at all, and with FileList and FileNames being parallel
vectors, they both had the same length so FileList could be used
for this test..
Use UniqueNameManager for document names and labels. This uses ad hoc
UniqueNameManager objects created on the spot on the assumption that
document creation is relatively rare and there are few documents, so
although the cost is O(n), n itself is small.
Use an ad hoc UniqueNameManager to name new DymanicProperty entries.
This is only done if a property of the proposed name already exists,
since such a check is more-or-less O(log(n)), almost never finds a
collision, and avoids the O(n) building of the UniqueNameManager.
If there is a collision an ad-hoc UniqueNameManager is built
and discarded after use.
The property management classes have a bit of a mess of methods
including several to populate various collection types with all
existing properties. Rather than introducing yet another such
collection-specific method to fill a UniqueNameManager, a
visitProperties method was added which calls a passed function for
each property. The existing code (e.g. getPropertyMap) would be
simpler if they all used this but the cost of calling a lambda
for each property must be considered. It would clarify the semantics
of these methods, which have a bit of variance in which properties
populate the passed collection, e.g. when there are duplicate names..
Ideally the PropertyContainer class would keep a central directory of
all properties ("static", Dynamic, and exposed by ExtensionContainer and
other derivations) and a permanent UniqueNameManager. However the
Property management is a bit of a mess making such a change a project
unto itself.
* Address the poor performance of the existing unique-name generation
As described in Issue 16849, the existing Tools::getUniqueName method
requires calling code to form a vector of existing names to be avoided.
This leads to poor performance both in the O(n) cost of building such a
vector and also getUniqueName's O(n) algorithm for actually generating
the unique name (where 'n' is the number of pre-existing names).
This has particularly noticeable cost in documents with large numbers
of DocumentObjects because generating both Names and Labels for each new
object incurs this cost. During an operation such as importing this
results in an O(n^2) time spent generating names.
The other major cost is in the saving of the temporary backup file,
which uses name generation for the "files" embedded in the Zip file.
Documents can easily need several such "files" for each object in the
document.
This update includes the following changes:
Create UniqueNameManager to keep a list of existing names organized in
a manner that eases unique-name generation. This class essentially acts
as a set of names, with the ability to add and remove names and check if
a name is already there, with the added ability to take a prototype name
and generate a unique form for it which is not already in the set.
Eliminate Tools::getUniqueName
Make DocumentObject naming use the new UniqueNameManager class
Make DocumentObject Label naming use the new UniqueNameManager class.
Labels are not always unique; unique labels are generated if the
settings at the time request it (and other conditions). Because of this
the Label management requires additionally keeping a map of counts
for labels which already exist more than once.
These collections are maintained via notifications of value changes on
the Label properties of the objects in the document.
Add Document::containsObject(DocumentObject*) for a definitive
test of an object being in a Document. This is needed because
DocumentObjects can be in a sort of limbo (e.g. when they are in the
Undo/Redo lists) where they have a parent linkage to the Document but
should not participate in Label collision checks.
Rename Document.getStandardObjectName to getStandardObjectLabel
to better represent what it does.
Use new UniqueNameManager for Writer internal filenames within the zip
file.
Eliminate unneeded Reader::FileNames collection. The file names
already exist in the FileList collection elements. The only existing
use for the FileNames collection was to determine if there were any
files at all, and with FileList and FileNames being parallel
vectors, they both had the same length so FileList could be used
for this test..
Use UniqueNameManager for document names and labels. This uses ad hoc
UniqueNameManager objects created on the spot on the assumption that
document creation is relatively rare and there are few documents, so
although the cost is O(n), n itself is small.
Use an ad hoc UniqueNameManager to name new DymanicProperty entries.
This is only done if a property of the proposed name already exists,
since such a check is more-or-less O(log(n)), almost never finds a
collision, and avoids the O(n) building of the UniqueNameManager.
If there is a collision an ad-hoc UniqueNameManager is built
and discarded after use.
The property management classes have a bit of a mess of methods
including several to populate various collection types with all
existing properties. Rather than introducing yet another such
collection-specific method to fill a UniqueNameManager, a
visitProperties method was added which calls a passed function for
each property. The existing code would be simpler if existing
fill-container methods all used this.
Ideally the PropertyContainer class would keep a central directory of
all properties ("static", Dynamic, and exposed by ExtensionContainer and
other derivations) and a permanent UniqueNameManager. However the
Property management is a bit of a mess making such a change a project
unto itself.
The unit tests for Tools:getUniqueName have been changed to test
UniqueNameManager.makeUniqueName instead.
This revealed a small regression insofar as passing a prototype name
like "xyz1234" to the old code would yield "xyz1235" whether or
not "xyz1234" already existed, while the new code will return the next
name above the currently-highest name on the "xyz" model, which could
be "xyz" or "xyz1".
* Correct wrong case on include path
* Implement suggested code changes
Also change the semantics of visitProperties to not have any short-circuit return
* Remove reference through undefined iterator
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Fix up some comments for DOxygen
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
==========================================================
Currently changes of name or type of properties in a property container are handled by:
void PropertyContainer::handleChangedPropertyName(Base::XMLReader &reader, const char * TypeName, const char *PropName)
void PropertyContainer::changedPropertyType(Base::XMLReader &reader, const char * TypeName, Property * prop)
There is no mechanism for handling property changes by extensions. Sometimes the solution is to explicitly call the extension
from the container. However, this is a breach of the SRP, as the container should not be in a position to decide whether the
extension needs or not handle property changes. The handling code of the container changes for two different reasons, for
adapting the container to a property change of its own, and for adapting that of a property of the extension.
Illustrating it with an example, following inheritance, it goes like this:
PropertyContainer => ExtensionContainer => TransactionalObject => ViewProvider
App::Extension => ViewProviderExtension
The extension is currently not notified by the ExtensionContainer that a property needs handling. So a change in a property of
a ViewProviderExtension needs code at the ViewProvider it was added to.
This commit provides a mechanism in ExtensionContainer to call the extensions so that they can handle property changes. This
functions:
virtual bool extensionHandleChangedPropertyName(Base::XMLReader &reader, const char * TypeName, const char *PropName);
virtual bool extensionHandleChangedPropertyType(Base::XMLReader &reader, const char * TypeName, Property * prop);
Containers should always call the base class for any unhandled property change. If a sub-class container of ExtensionContainer
handles property changes itself, but not the ones of the extensions, this call to the base class ultimately ensures that if the
property was not handled by the container hierarchy, any extension is given an opportunity to handle it.
Some examples:
* A container handles the extension property change or its own:
void ContainerSubClass::handleChangedPropertyType(...)
{
if (prop == &PropertyOfExt) {
}
else if (prop == &PropertyOfCont) {
}
else {
ContainerBaseClass::handleChangedPropertyType(...);
}
}
* A container and the extension handle their own:
void ContainerSubClass::handleChangedPropertyType(...)
{
if (prop == &PropertyOfCont) {
}
else {
// This will call ExtensionContainer::handleChangedPropertyType
ContainerBaseClass::handleChangedPropertyType(...);
}
}
bool ExtensionSubClass::extensionHandleChangedPropertyType(...)
{
if (prop == &PropertyOfCont) {
return true;
}
return false;
}
DocumentObject:
* getSubObject(): the most important API for Link to work with
hierarchies. The function is a inspired from and replaces the
getPySubObjects(). It returns a child object following a dot separated
subname reference, and can optionally return accumulated
transformation, and/or a python object of the refered
sub-object/element. The default implementation here is to look for
link type property, and search for the referenced object. This patch also
include other specialized implementation of this API, such as
(GeoFeature)GroupExtension (through extensionGetSubObject()),
PartDesign::Body, and so on. A link type object is expected to
call the linked object's getSubObject() for resolving.
* getSubObjectList(): helper function to return a list of object
referenced in the given subname.
* getSubObjects(): return a list of subname references of all children
objects. The purpose of this function is similar to
ViewProvider::claimChildren(). Container type object is expected to
implement this function. The reason it returns subname references
instead of just object is to allow the container to skip hierarchies.
For example, the Assembly3 container uses this to skip the constraint
and element group.
* getLinkedObject(), obtain the linked object, and optionally with the
accumulated transformation. It is expected to return a linked object
or the object itself if it is not a link. In case there are multiple
levels of linking involved, this function allows the caller to retrieve
the linked object recursively.
* hasChildElement(), set/isElementVisible(), controls the children
visibility for a group type object. Because the child object may be
claimed by other objects, it is essential to have independent control
of children visibilities. These APIs are designed to abstract how
group manages the child visibility. For performance reason, these
function are meant to control only the immediate child object.
* resolve(), helper function to parse subname reference and resolve the
final object, and optionally the immediate parent of the final object,
the final object reference name (for calling `set/isElementVisible()`),
and the subname reference if there is one.
* touch(), add optional argument 'noRecompute' for better backward
compatibility with the NoRecompute flag. By default, touch() causes
recompute unless noRecompute is true
* signalChanged/signalBeforeChange, two new signal for tracking changes
of a specific object.
* getViewProviderNameOverride(), return a string of the view provider
type of this object. This allows Python class to override the view
provider of an object. This feature will be used by ViewProviderLink
which is designed to work with any object that has LinkBaseExtension.
* canLinkProperties(), will be used by Gui::PropertyView to display
linked object properties together with the object's own properties.
* redirectSubname(), will be used by Gui::Tree to allow an object to
redirect selection to some other object when (pre)selected in the tree
view.
* Visibility, new property serve as the same purpose as view provider
property of the same name. It is added here so that App namespace
code can check for visibility without Gui module. This is useful,
for example, when constructing a compound shape of a container that
respects the children visibility.
* (has)hasHiddenMarker(), return or check for a special sub-element
name used as marker for overriding sub-object visibility. Will be
used by Gui::ViewProvider, it is put here for the same reason as
adding Visibility property.
* getID(), return object internal identifier. Each object is now
assigned an integer identifier that is unique within its containing
document.
Document:
* ShowHidden, new property to tell tree view whether to show hidden
object items.
* signalTouchedObject, new signal triggered when manually touch an
object when calling its touch() function
* getObjectByID(), get object by its identifier
* addObject() is modified to allow overriding view provider
* has/getLinksTo(), helper function to obtain links to a given object.
Application:
* checkLinkDepth(), helper function to check recursive depth for link
traversal. The depth is checked against the total object count of
all opened documents. The count (_objCount) is internally updated
whenever object is added or removed.
* has/getLinksTo(), same as Document::has/getLinksTo() but return links
from all opened documents.
GroupExtension/OriginGroupExtension/DatumFeature/DatumCS/Part::Feature:
implement sepcialized getSubObject/getSubObjects().
FreeCADs property system utilises some pointer math to calculate the offset between
property and base class. Due to virtual inheritance of th ePropertyContainer the memory
layout has been changed to rather random, which has lead to crashes dependend on the
order of object initialisation.
The solution is to not make PropertyContaner virtual but a class below, Base::Persitance.
Then the memory layout is random for Persistance, but it is perfectly aligned for the
base class chains from PropertyContainer onwards as well as from Extension onwards.
Hence the proeprty system was changed to take the offset always from those two.
