An empty block after a conditional can be a sign of an omission and can decrease maintainability of the code.
Such blocks should contain an explanatory comment to aid future maintainers.
==================================================================
It is possible to bypass SketchObject in modifying geometry and constraints. Like in here:
https://forum.freecadweb.org/viewtopic.php?f=3&t=41326&start=20#p408409
This leads to unexpected behaviour and even crashes.
With this commit the new mechanism of constraint indices check is leveraged in cases not involving SketchObject operations (aka managed operations).
Direct assignment of properties from Python (sketcher unmanaged operations), undergo this extra indices check.
When indices in constraints are outside the geometry range, the constraints are shown as empty and the error is shown in the report window.
======================================================
PropertyConstraintList is provided with the ability to check its constraints indices against
a minimum and a maximum, and set an invalidindex status.
In this status, the getValues returns an emptylist, as in the case with invalid geometry types.
================================================================================
Solves the part of this described bug relating to reference constraints affecting the diagnosis of redundant constraints:
https://forum.freecadweb.org/viewtopic.php?p=410195#p410195
==============================================
Take advantage of PropertyGeometryList setValues() move overload in order to make code more readable and prevent
memory leaks (mostly by inadvertedly not deleting cloned geometry and constraints).
PropertyGeometryList and PropertyConstraintList are vectors of heap allocated pointers. Copying a vector
makes a shallow copy, not a deep copy (the pointers are the same in the copy).
For property management, setValues() function taking a const reference effectively make a deep copy of all
pointed objects. This means that heap allocated pointers of the client class passed to these functions must be
released by the client. While this sounds sensible, forgetting to is easy. In the cases where the developer
remembered to release these pointers, extra code is needed just for memory management.
This commit does not seek a substantial performance increase that would justify rewritting the code, although code
may be slightly faster sometimes.
Functions where setValues() is conditional are not changed to move semantics, as it makes no sense to make a deep copy to sometimes
perform a second deep copy later on. This code still uses const ref setValues().
CHECKS performed to refactored functions with this commit:
1) That the vector is NOT used after moving its content.
2) That whereever there is a clone(), there must be EITHER
-a std::move if using rvalue setValues()
OR
- a delete to free the heap memory after setValues if using the const ref setValues()
3) That memory is released if an exception occurred.
N.B.: A couple of memory leaks are fixed in this commit too.
==============================================================================
Inhibit ViewProviderSketch updateData with invalid data (internal transaction).
Trigger update internally for internal transactions in SketchObject via touched()
so as to trigger updateData.
This leads to a reduction of updateData calls.
===================================================================================
On changing the geometry property (for example from Python), the constraints geometry indices was not rebuild in order to avoid
redundant and unnecessary rebuilds. However, this might cause crashes, as the status of the sketch (or its properties) may be invalid.
It also refactors into OnChanged common functionality.
This commit does NOT solve that the user may be inserting invalid geometry indices to the First/Second/Third of Constraints (invalid input).
Only makes sure that geometry indices (geometry types) of PropertyConstraintList match the geometry.
Solution:
1. Force the rebuild of the constraint geometry indices upon assignment of new Geometry.
2. Force the rebuild of the constraint geometry indices upon assigment of constraints, if they result in invalid geometry indices.
3. Introduce the concept of internal transaction to avoid those rebuilds, checks and updates in case of an ongoing internal transaction,
thereby preventing them as it was done before introducing 1 and 2 (in the case of SketchObject internal transactions).
==================
https://forum.freecadweb.org/viewtopic.php?p=387303#p387303
1. Trim had a bug that the type of the constraint on the second point was equal to the first one regardless of the situation.
2. Trim did not have support for checking whether points were close to the edge and relied on preexisting constraints.
std::move is redundant when it is used to return a local object from a function (eg return std::move(local)): indeed, returning a local object from a function implicitly moves it. Moreover using std::move this way
See https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rf-return-move-local
However, in order to avoid -Wreturn-std-move as well, a Base object is move-constructed
from Derived when required.
Found via `codespell -q 3 -L aci,ake,aline,alle,alledges,alocation,als,ang,anid,ba,beginn,behaviour,bloaded,byteorder,calculater,cancelled,cancelling,cas,cascade,centimetre,childs,colour,colours,commen,currenty,dof,doubleclick,dum,eiter,elemente,feld,freez,hist,iff,indicies,initialisation,initialise,initialised,initialises,initialisiert,ist,kilometre,lod,mantatory,methode,metres,millimetre,modell,nd,noe,normale,normaly,nto,numer,oder,orgin,orginx,orginy,ot,pard,pres,programm,que,recurrance,rougly,seperator,serie,sinc,strack,substraction,te,thist,thru,tread,uint,unter,vertexes,wallthickness,whitespaces -S ./.git,*.po,*.ts,./ChangeLog.txt,./src/3rdParty,./src/Mod/Assembly/App/opendcm,./src/CXX,./src/zipios++,./src/Base/swig*,./src/Mod/Robot/App/kdl_cp,./src/Mod/Import/App/SCL,./src/WindowsInstaller,./src/Doc/FreeCAD.uml`
