Wouldn't it be better and more intuitive for the user if that content was displayed but with an error message?logari81 wrote: the Constraints list detects a mismatch to the last accepted geometry and hides all its content.
Reorient sketch loses constraints
Forum rules
and Helpful information
and Helpful information
IMPORTANT: Please click here and read this first, before asking for help
Also, be nice to others! Read the FreeCAD code of conduct!
Also, be nice to others! Read the FreeCAD code of conduct!
Re: Reorient sketch loses constraints
Re: Reorient sketch loses constraints
I believe it is a bug and the cause (my previous post wasn't very clear) appears to be in SketchObject::delConstraintsToExternal.
The offending line is in bold below. My understanding is the the First and Second elements of a Constraint hold either an integer index into the geometry list (0, 1, 2 etc), -1 or -2 for horizontal and vertical axes or an index into the external geometry list (-3, -4, -5, -6 etc) OR -2000 if the particular constraint only references one geometry (in the case of Second, a similar usage for Third).
The code below deletes any constraint that has First or Second <= -3. All single geometry constraints (horizontal, vertical, line lengths , radii) have Second = -2000 so they get deleted in error.
int SketchObject::delConstraintsToExternal()
{
const std::vector< Constraint * > &constraints = Constraints.getValues();
std::vector< Constraint * > newConstraints(0);
int GeoId = -3;
for (std::vector<Constraint *>::const_iterator it = constraints.begin();
it != constraints.end(); ++it) {
if ((*it)->First > GeoId && (*it)->Second > GeoId) {
newConstraints.push_back(*it);
}
The offending line is in bold below. My understanding is the the First and Second elements of a Constraint hold either an integer index into the geometry list (0, 1, 2 etc), -1 or -2 for horizontal and vertical axes or an index into the external geometry list (-3, -4, -5, -6 etc) OR -2000 if the particular constraint only references one geometry (in the case of Second, a similar usage for Third).
The code below deletes any constraint that has First or Second <= -3. All single geometry constraints (horizontal, vertical, line lengths , radii) have Second = -2000 so they get deleted in error.
int SketchObject::delConstraintsToExternal()
{
const std::vector< Constraint * > &constraints = Constraints.getValues();
std::vector< Constraint * > newConstraints(0);
int GeoId = -3;
for (std::vector<Constraint *>::const_iterator it = constraints.begin();
it != constraints.end(); ++it) {
if ((*it)->First > GeoId && (*it)->Second > GeoId) {
newConstraints.push_back(*it);
}
Re: Reorient sketch loses constraints
yes, your diagnosis seems right to me, good catch.
Re: Reorient sketch loses constraints
Hi wmayer,
Thanks for looking at that patch.
I'm also using the below patch in void SketchObject::onChanged(const App::Property* prop).
This patch cleans up a change of sketch support.
Without it, when sketch support changes, the external geometry list is wiped but nothing is done with the constraints list so there could be invalid constraints.
This clears the constraints before deleting each external geometry.
EDIT:
Should add that the patch deletes external geometry whenever the support changes. Previous behaviour only deleted external geometry if the support was null. This didn't make much sense because the external geometry was very unlikely to be valid after any support change.
Thanks for looking at that patch.
I'm also using the below patch in void SketchObject::onChanged(const App::Property* prop).
This patch cleans up a change of sketch support.
Without it, when sketch support changes, the external geometry list is wiped but nothing is done with the constraints list so there could be invalid constraints.
This clears the constraints before deleting each external geometry.
EDIT:
Should add that the patch deletes external geometry whenever the support changes. Previous behaviour only deleted external geometry if the support was null. This didn't make much sense because the external geometry was very unlikely to be valid after any support change.
Code: Select all
diff --git a/src/Mod/Sketcher/App/SketchObject.cpp b/src/Mod/Sketcher/App/SketchObject.cpp
index 0626294..86bf07a 100644
--- a/src/Mod/Sketcher/App/SketchObject.cpp
+++ b/src/Mod/Sketcher/App/SketchObject.cpp
@@ -1585,10 +1585,9 @@ void SketchObject::onChanged(const App::Property* prop)
// make sure not to change anything while restoring this object
if (!isRestoring()) {
// if support face was cleared then also clear the external geometry
- if (!Support.getValue()) {
- std::vector<DocumentObject*> obj;
- std::vector<std::string> sub;
- ExternalGeometry.setValues(obj, sub);
+ delConstraintsToExternal();
+ for (int i=0; i < getExternalGeometryCount(); i++) {
+ delExternal(0);
}
}
}