=================================================================================
Apart from simplifying code, the allocated memory is not managed by the lifetime of the unique pointer owned by the GeometryFacade, preventing memory leaks.
================================================================================
GeoEnum static members are used in constexpr in GeoListId.
Previous code worked because the static members where inside the same translation unit.
This code:
1. Declares the static members to use them as constant expressions with the value
2. Definition of the static members (odr-used) shall not contain an initializer.
When new control points are created in a b-spline in a sketch, do not constrain
their weights to be equal to the first, unless they are already equal. This way
if we have unequal weights to begin with and OCC has computed the appropriate
weights so that the new spline is closer to the original, the weights are not
disturbed.
==============================
- Improve GeoList to support GeoElementId input
- Support VertexId to GeoElementId conversion
- GeoListModel gets separate getGeometry and getGeometryFacade naked pointers
- Sketcher: GeoList - make index mappings mutable
- Improve documentation
===================================================
This commit is an independent refactor of the identifications used at Sketcher level.
It introduces a new type "GeoElementId" as a combination of GeoId and PointPos.
It moves the Undefined GeoId, previous Constraint::GeoUndef to GeoEnum, together with all
other fixed values of GeoIds.
========================================================================
As GeometryFacade is provided with more sketcher specific functionality, the demand
for to get GeometryFacades instead of Part::Geometry object increases too. This
addition reflects this increase of demand.
==============================================
Separate struct into own file to avoid having to include all the dependencies of SketchObject in other code that relies on GeoEnum.
==============================
Class for managing internal and external geometry as a single object. This is a light-weight alternative to
passing the whole SketchObject.
It reflects the format used in getCompleteGeometry of SketchObject and Sketch solver facade class, while providing
several convenient conversion functions to map indices.
Internal and external geometries are present in a single geometry vector one after the other.
The index of the geomlist (all layers) and the GeoId can be converted from each other at needed
using the member fuctions (and sometimes the static member functions).
Internal implementation is as a template GeoListModel<T>.
The following types are instantiated. Specialisation is provided where necessary.
GeoList = GeoListModel<Part::Geometry *>;
GeoListFacade = GeoListModel<std::unique_ptr<const Sketcher::GeometryFacade>>;
This enables to use the lighter GeoList were sufficient, while enabling off-the-shelf replacement
when switching to a GeoListFacade is necessary.
LGTM complains about using continue statements inside a loop
whose condition is always false. In addition, the C++ core guidelines
recommend against using the do...while construct, and in this case it
was really serving as a goto, just hiding the actual goto keyword.
This commit replaces the loop and continue structure with simple
conditionals.
=============================================
This commit is directed to external functionality of the split() function.
1. getAppliedConstraints renamed to getConstraintIndices
This is just for clarity being a general function
2. SwapInvolvedGeometry functionality moved to Constraint class
Why?
i. Because it is a specific operation on a constraint, it must not be
a public function, as it does not define interface of the Sketch.
ii. It could be a lambda or a private utility function, but them it would not be reusable.
iii. It could be part of a helper class, but then, it is would be less reusable.
3. renaming of the flag passed to transferConstraints function
======================================
* Refactor the code for all GeomTrimmedCurve and non-periodic BSplines in a single block, adding
support for arcs of hyperbola, parabola and B-Splines.
* Refactor the code for periodic curves (circle, ellipse) in a single block, adding support for
periodic B-Splines.
* Add support for trimming limited by external geometry
* Trim deletes a geometry if intersections are detected and nothing would be left after trimming
* Trim deletes a geometry if no intersection are detected
Bonus:
* Function addConstraint moving the constraint instead of cloning it again
* SketchObject::seekTrimPoints as a wrapper of Part2DObject::seekTrimPoints providing
a correct handling of GeoId indices.
* Helper functions addConstraint/createconstraint to create new constraints and move them
into the Constraint property
* New getGeometry with templated return type defaulting to Part::Geometry.
====================================================================
When the ellipse to be projected and the sketch plane are parallel, the original code
by shermelin provided for a translation of the original ellipse, which would be the best solution
if it weren't because the Sketcher, internally, works under the assumption of a normal vector to the
sketcher plane being (0,0,1). If the original ellipse is parallel to the sketch plane, but the sketch
plane is not the XY plane, the copy and translation would result in a ellipse not in the XY plane of the
Sketcher. Then the sketcher internals will not properly consider its dimensions.
The solution applied here is to default to the general method for non-parallel planes.
It solves:
https://forum.freecadweb.org/viewtopic.php?f=3&t=55284#p477522
========================================================
Users chennes and hyarion made me aware of this covereity issue:
Fixes Coverity: geoit can be end() when dereferenced
https://github.com/FreeCAD/FreeCAD/pull/4429/files#
When analysing the block where the dereferrencing appears, it
appears that it is a left-over that no longer makes sense:
- The algorithm classifies block constraints into those that are
not affected by any other driving constraint and those that are
affected by other driving constraints.
- The offending block deals with internal aligned geometry, thus
per definition has a driving internal alignment constraint, for which
the previous block already set the need of post-analysis.
- No matter what, the geometries, the complex one and the internal one
will have at least the driving internal alignment constraint, so they
cannot become "not affected by any other driving constraint".
- If the geometry had a block constraint on it, it was already added for
post-analysis in the previous block. If it did not have one block constraint,
the fact that it is internal aligned geometry is an irrelevant consideration.
Probably there was a point during development when this made sense, but with
the current post-analysis, it does not appear to make sense anymore. So the
block was removed.
This commit adds a unit test for blocked geometry (new block constraint).
Fixes Coverity issue:
CID 316539 (#1 of 1): Uninitialized scalar field (UNINIT_CTOR)
uninit_member: Non-static class member lastHasPartialRedundancies is not initialized in this constructor nor in any functions that it calls.
=================================================
Knot position is not calculated by the solver, but by OCCT when updating the
b-spline to conform to given pole positions, as mandated by the solver. Before
this commit, all constraints driving and non-driving operating on the knots required
and extra solve (from advanced solver dialog, or from the Python console), or a recompute
to be recomputed.
This commit introduces transparently re-solving at Sketch.cpp level if B-Splines are present,
so that when the Sketcher mandated solve returns, the geometry is fully solved.
========================================
The geometry state stored in the geometryFacade is modified following a mutable model
(without setting the Geometry property on Constraint change), in order to avoid coupling
the addition/removal of a constraint with a change of the Geometry Property.
This design decision however interferes with the ability of the Geometry property to restore
the correct geometry state upon redo/undo.
While such a situation is rare in the case of Internal Alignment geometry, because constraint
addition/removal is performed with the corresponding geometry addition/removal (within the same
transaction. That is not the case with the Block constraint (or another future general case where
the geometry state may be applied).
This commit leverages the synchronisation mechanism already in use for non-properties (e.g. external geometry or
vertex indices) to check and synchronise geometry state upon undo/redo and restore.
Bonus:
- addGeometryState is refactored to separate the checking logic from the setting logic.
