========================================================
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).
=================================================
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.
========================================
Previous versions relied on a heuristic that proved insufficient for cummulative use of the Block constraint.
The effect is that Block constraints stopped blocking, which is a major bug, as it lead to inadvertedly moving
geometry that was supposed to be blocked.
Fixes:
https://forum.freecadweb.org/viewtopic.php?f=13&t=53515&start=30#p461215
(Thanks Chaospilot)
Know problems with old block constraint (v0.18):
1. If driving constraints were present, they were ignored if inserted before the block constraint (to avoid
redundancy/conflicting). They resulted in
Principles of Working of the new block constraint:
1. Handling of the new block constraint is based two processes, a pre-analysis and a post-analysis. Pre-analysis
works *before* diagnosing the system in the solver. Post-analysis works *after* diagnosing the system in the solver.
2. Pre-analysis is directed to detect geometries affected *exclusively* by a block constraint. This is important
because these geometries can be pre-fixed when creating the solver geometry and constraints before GCS::diagnose()
via initSolution() AND because if no other constraint affects the geometry, the geometry parameters won't even appear
in the Jacobian of GCS, so they won't be reported as dependent parameters (for which post-analysis would be of no use).
3. Post-analysis is directed to detect Geometries affected *not only* by a block constraint. This is important
because pre-fixing these geometries would lead to redundant constraints. The post-analysis, enables to fix just the
parameters that fulfil the dependacy groups.
4. Post-analysis basically identifies which parameters shall be fixed to make geometries having blocking constraints
fixed, while not leading to redundant/conflicting constraints. These parameters must belong to blocked geometry. This
is, groups may comprise parameters belonging to blocked geometry and parameters belonging to unconstrained geometry. It
is licit that the latter remain as dependent parameters. The former are referred to as "blockable parameters". Extending
this concept, there may be unsatisfiable groups (because they do not comprise any bloackable parameter), and it is the
desired outcome NOT to satisfy such groups. It must be emphasised that there is not a single combination of fixed parameters
from the blockable parameters that satisfy all the dependency groups. However:
1) some combinations do not satisfy all the dependency groups that must be satisfied (e.g. fixing one group containing
two blockable parameters with a given one may result in another group, fixable only by the former, not to be satisfied).
This leads, in a subsequent diagnosis, to satisfiable unsatisfied groups.
2) some combinations lead to partially redundant constraints, that the solver will silently drop in a subsequent diagnosis,
thereby reducing the rank of the system fixing less than it should.
5. The implementation rationale is as follows:
1) The implementation is on the order of the groups provided by the QR decomposition used to reveal the parameters
(see System::identifyDependentParameters in GCS). Zeros are made over the pilot of the full R matrix of the QR decomposition,
which is a top triangular matrix.This, together with the permutation matrix, allow to know groups of dependent parameters
(cols between rank and full size). Each group refers to a new parameter not affected by the rank in combination with other free
parameters intervening in the rank (because of the triangular shape of the R matrix). This results in that each the first column
between the rank and the full size, may only depend on a number of parameters, while the last full size colum may dependent on
any amount of previously introduced parameters.
2) Thus the rationale is to start from the last group (having **potentially** the larger amount of parameters) and selecting as blocking
for that group the latest blockable parameter. Because previous groups do not have access to the last parameter, this can never
interfere with previous groups. However, because the last parameter may not be a blockable one, there is a risk of selecting a parameter
common with other group, albeit the probability is reduced and probably (I have not demonstrated it though and I am not sure), it
systematically leads to the right solution in one iteration.
GCS: Change dependency group from std::set to std::vector to prevent reordering of parameters.
=============================================================
Fixes:
https://forum.freecadweb.org/viewtopic.php?f=8&t=53466&p=460513#p460270
When a parameter belonging to a blocked geometry is present in several
dependency groups and there are other fixed parameters one or more of
the dependency groups, it is not enough to remove that parameter, from
the one or more dependency groups. The removal of a parameter may only
satisfy one dependency group. Removing a single parameter creates a
new different dependency group.
Solution:
If the first blocked parameter in a group is already set for removal,
continue searching for other blocked parameters, until one not searched
for removal is found.
======================================================
With the new solver ability to identify groups of dependent parameters, even with SparseQR, it is now possible to properly implement Blocked constraint (blocked geometry state/mode).
This comes at the cost of two consecutive diagnosis (4 QR decompositions, running in parallel by pairs, so the double of time than when not using any blocked constraint).
A first diagnosis identifies the groups of dependent solver parameters.
If any of these groups comprises a parameter affected by a blocked constraint, the parameter is fixed. As only one parameter from each group is fixed, there are no (partly) redundant parameters involved.
Then the new adapted system is solved.
=========================================================================================================
Former mechanism with hasDependentParameters is not flexible enough for the new kind of information.
This commit further enhances the calculation of dependent parameters and dependent parameter groups by
caching the parameter-geoelement (GeoId, PointPos) relationship during geometry creation.
This commit provides traditional information whether a parameter is dependent via SolverGeometryExtension. New
enhanced information about groups of dependent parameters are available via the Sketch API.
=============================================================================================
This commits removes the Geometry construction data member and adapts sketcher code to use
GeometryFacade to access construction information via the SketchGeometryExtension.
========================================================================
Until now BSpline poles were circles relying on physical length units. This lead to several
problems:
- While the BSpline weight follows the circle size, weights do not have length units, but are adimensinal
- As representation of the BSpline depends on the physical size of the circle, the numerical value to be
set to a pole circle differs from the numerical value of the weight.
The present commit:
1. Separates pole circle representation (physical size), from the numerical value used in the radius constraint,
so that the value in the constraint is the weight, the value representation is a factor of the weight value (in this
commit is getScaleFactor(), but this will change in the next commit). Dragging accounts for this scale factor too.
2. While Radius constraint button is used to constraint a B-Spline weight as before, this creates a Weight constraint,
which is a new type of constraint. This is done so that the value is truly adimensional and is so presented in all kind
of editors that rely on the units indicated by the constraint. It is obviously also shown as adimensional (thus without units),
in the 3D view and in the datum dialogs.
3. Because the circle of the pole of a B-Spline is not a geometric circle, but a graphical representation of the pole and how
it affects the corresponding B-Spline, constraint creation commands are limited so that no point on object, tangent, perpendicular
or SnellLaw constraints can be created on a B-Spline weight circle. This is also the case for the Diameter constraint, which won't
accept the circle. Equality constraints work either on only circles or only weights, but not on a mixture of them.
Bonus: This commit fixes a bug in master, that using the select equality constraint then click in two geometric elements mode, you
could make a circle equal to an ellipse resulting in malformed solver constraints.
========================================================================================
Until this commit, B-Spline knots were stored, and checked at solver level as sketch points GeomPoints having the
construction status set to true.
This behaviour is removed to eventually enable normal non-construction points to define the shape out of edit mode.
This commit leverages the new mechanism for the solver interface.
- Support for addition of driving information to GCS constraints
- Support for keeping track of value parameters of driven constraints
- Support for getting which QR algoritm is being used at GCS level
===========================================================================
Block constraint is naturally redundant/conflicting with almost any other constraint applied to an edge (certainly with constraints operating only on that element).
As such, a block constraint will have very little applicability in absence of a way to handle this redundancy/conflicting. For example, in the case of a BSpline all its
internal geometry (construction circles on poles and its constraints) would have to be removed before locking. This would mean that it is not possible to define the BSpline shape
and then block it, as when removing the internal geometry the shape would be lost. In other cases, like temporally blocking to avoid that a part of the sketch moves while performing some
operation with the idea of afterwards unblocking it, it would mean removing all constraints, to add the block constraint, then perform the action, then remove the block constraint and manually
constraint it again.
Handling this situation in a user expected way means ignoring certain constraints (those causing the redundancy/conflicting), so that the solver is not aware of them and does not complain. However,
generally ignoring those constraints has a negative effect, in that constraints applied by the user on already blocked geometry, or constraints that otherwise lead to a conflicting or redundant
situation as a consequence of actions (further constraining) after the Block constrain is applied are ignored, thereby not properly informing the user of this situation, which is undesirable.
This new mechanism takes account on the position of the constraints relative to the involved blocked constraint(s). As such, redundant/conflicting constraints that were added before the Block
constraint are ignored, whereas those constraints that lead to a redundant/conflicting situation and added AFTER the block constraint was already in place, those are not ignored and are reported
accordingly.
