If an object has a link to itself it may cause a stackoverflow
in several cases:
* If the method claimChildren3D() returns a list containing the
object of the view provider then Document::handleChildren3D()
will add a SoGroup to itself as a child. This will result into
a stackoverflow as soon as an action traverses the scene.
* If the method claimChildren() returns a list containing the
object of the view provider then DocumentItem::createNewItem()
causes an infinite loop with DocumentItem::populateItem()
Solution:
* Inside Document::handleChildren3D() avoid to add a SoGroup to itself
* In this specific case fix ViewProviderCoordinateSystem::claimChildren()
to avoid a cyclic dependency
Hint: Since PR 18126 FreeCAD is vulnerable for this problem.
This fixes issue 19682
Historically, `App::Document::recompute()` ran entirely on the **main**
(GUI) thread and directly emitted `signalBeforeRecompute()`.
* Add-ons like **Assembly3** and others depend on that signal for
setup/teardown hooks before any recompute work begins.
* After offloading `recompute()` into a background worker thread to keep
the UI responsive, calling `signalBeforeRecompute()` directly from the
worker would break thread-affinity rules and silently break
compatibility with those add-ons.
**Solution**
1. **Introduce a generic hook** (`PreRecomputeHook`) in
**App::Document**:
* A `std::function<void()>` that, if set, is invoked at the very
start of `recompute()`.
* Core code stays Qt-free—only knows to call a callback if one
exists.
2. **Wire up the hook in `Gui::Document`**:
* In the GUI wrapper’s constructor, install a hook that calls
`callSignalBeforeRecompute()`.
* `callSignalBeforeRecompute()` uses `QMetaObject::invokeMethod(...,
Qt::BlockingQueuedConnection)` to enqueue `signalBeforeRecompute()` on
the GUI thread and **block** the worker until it completes.
* If already on the GUI thread, it simply calls the signal directly.
3. **Maintain add-on compatibility**:
* From the add-on’s perspective nothing changes—they still receive
`signalBeforeRecompute()` on the main thread before any recompute work.
* Internally, the recompute body now runs on a worker thread,
improving UI responsiveness without breaking existing hooks.
**Result**
* **Recompute** remains fully backward-compatible for add-ons like
Assembly3.
* **UI thread** still handles all GUI-related signaling.
* **Worker thread** performs the actual heavy lifting, unblocked only
once the GUI is primed and all pre-recompute signals have been
delivered.
Previously the code defined compatiblity usings in `QtOpenGL.h` header,
which I think was added for backwards compatiblity with previous Qt
OpenGL widgets.
As far as I can tell, this is not necessary anymore, and can be cleaned
up.
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.
This adds isNullOrEmpty string helper that cheks if string is... well
null or empty. It is done to improve readability of the code and better
express intent.
* 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>
* MeasureGui: Store measure type in TaskMeasure
* MeasureGui: Avoid adding measurement to document during command interaction
* [Gui] Add check for document in VPDocumentObject::getActiveView
* MeasureGui: Track the document when adding objects
* MeasureGui: Cleanup python measurement creation
* [Gui] Add isAnnotationViewProvider method
* [Gui] Check if viewprovider is added as an annotation in getActiveView
* [Gui] Add takeAnnotationViewprovider method to Gui::Document
* [Gui] Make addViewProvider public
* [MeasureGui] Add existing view provider to document when storing measurement
* [MeasureGui] Fix invocation of initial label placement
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
This patch substitutes by isAttachedToDocument() (almost) everywhere where
getNameInDocument() is used for this purpose.
The very few places not touched by this patch demand a (just a little) less trivial change.
When we change the returning type of getNameInDocument() to std::string,
those places will be easily found, because they shall generate a compiler error
(converting std::string to bool).
Rationale:
The fact that getNameInDocument() return nullptr to indicate
that the object is not attached to a document is responsible for lots of bugs
where the developer does not check for "nullptr".
The idea is to eliminate all those uses of getNameInDocument() and, in the near future,
make getNameInDocument() return always a valid std::string.
* Add unit system to Project Information and store with document.
* Remove the project unit system
* Restore correct document activation signalling to fix test fail
* Remove commented out dead lines
* Restore ignore option for project unit schemas
* Whitespace fix
* Refresh after changing units
* Remove field label
* Property editor changes applied to unit system
