Reorient sketch loses constraints

Post here for help on using FreeCAD's graphical user interface (GUI).
Forum rules
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!
User avatar
bejant
Posts: 6076
Joined: Thu Jul 11, 2013 3:06 pm

Re: Reorient sketch loses constraints

Postby bejant » Mon Apr 14, 2014 4:53 pm

logari81 wrote: the Constraints list detects a mismatch to the last accepted geometry and hides all its content.
Wouldn't it be better and more intuitive for the user if that content was displayed but with an error message?
chrisf
Posts: 212
Joined: Fri Jan 03, 2014 10:20 am

Re: Reorient sketch loses constraints

Postby chrisf » Mon Apr 14, 2014 5:03 pm

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);
}
logari81
Posts: 654
Joined: Mon Jun 14, 2010 6:00 pm

Re: Reorient sketch loses constraints

Postby logari81 » Mon Apr 14, 2014 5:31 pm

yes, your diagnosis seems right to me, good catch.
wmayer
Site Admin
Posts: 15756
Joined: Thu Feb 19, 2009 10:32 am

Re: Reorient sketch loses constraints

Postby wmayer » Thu May 01, 2014 4:05 pm

chrisf
Posts: 212
Joined: Fri Jan 03, 2014 10:20 am

Re: Reorient sketch loses constraints

Postby chrisf » Thu May 01, 2014 7:20 pm

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.

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);
             }
         }
     }
wmayer
Site Admin
Posts: 15756
Joined: Thu Feb 19, 2009 10:32 am

Re: Reorient sketch loses constraints

Postby wmayer » Fri May 02, 2014 10:05 am