Undo in Sketcher creates inconsistent 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!
abdullah
Posts: 3504
Joined: Sun May 04, 2014 3:16 pm

Re: Undo in Sketcher creates inconsistent constraints

Postby abdullah » Thu May 21, 2020 5:25 am

chrisb wrote:
Wed May 20, 2020 8:22 pm
Glad to see it's reproducable, and much more glad to see you having a look!
I am still debugging. The funny thing is that is you name your constraints, the thing works perfectly. Just name your angle "myangle" and your constraint "myconstraint" and it works... keep debugging...
chrisb
Posts: 25204
Joined: Tue Mar 17, 2015 9:14 am

Re: Undo in Sketcher creates inconsistent constraints

Postby chrisb » Thu May 21, 2020 5:48 am

I'm curious about the solution of this riddle.
wmayer
Site Admin
Posts: 15966
Joined: Thu Feb 19, 2009 10:32 am

Re: Undo in Sketcher creates inconsistent constraints

Postby wmayer » Sat May 23, 2020 10:46 am

<Exception> ObjectIdentifier.cpp(615): Py::Object App::ObjectIdentifier::Component::get(const Py::Object &) const -- No attribute named 'RadiusFrontAussen'
in property binding 'Constraints'
38.5191 <App> Document.cpp(3693): Failed to recompute undo3#Sketch: No attribute named 'RadiusFrontAussen'
in property binding 'Constraints'
<Exception> ObjectIdentifier.cpp(615): Py::Object App::ObjectIdentifier::Component::get(const Py::Object &) const -- No attribute named 'RadiusFrontAussen'
in property binding 'Constraints'
40.2105 <App> Document.cpp(3693): Failed to recompute undo3#Sketch: No attribute named 'RadiusFrontAussen'
in property binding 'Constraints'
To see this error message it suffices to delete the constraint and then click on the Update / Refresh button. The problem is that one of the constraints still references the name of the deleted constraint. The question is how to handle this case. IMO, the best way is to improve the self-diagnosis of the sketch and mark the constraint as invalid that references the deleted constraint.

The reason why the sketch breaks after a few undo/redo steps seems quite unrelated to the former issue. What I have found out so far is that inside PropertyConstraintList::setPathValue the wrong index for the angle constraint is determined.

Code: Select all

size_t index = c1.getIndex(_lValueList.size());
returns index=2 for the failing case (which is the tangent constraint) but it should be index=1.

Edit 1: It determines the wrong index value because the wrong ObjectIdentifier is passed to PropertyConstraintList::setPathValue

Edit 2: It passes the wrong ObjectIdentifier because the ExpressionEngine property of the Sketch changes
After loading the project

Code: Select all

App.ActiveDocument.Sketch.ExpressionEngine
[('Constraints[1]', '<<FolienVorlage>>.Constraints.distAussen * 360 / 2 / pi / .Constraints.RadiusFrontAussen')]
After deleting the radius constraint it gives
[('Constraints[0]', '<<FolienVorlage>>.Constraints.distAussen * 360 / 2 / pi / .Constraints.RadiusFrontAussen')]
Undo
[('Constraints[1]', '<<FolienVorlage>>.Constraints.distAussen * 360 / 2 / pi / .Constraints.RadiusFrontAussen')]
Redo
[('Constraints[0]', '<<FolienVorlage>>.Constraints.distAussen * 360 / 2 / pi / .Constraints.RadiusFrontAussen')]
After Undo again it suddenly becomes
[('Constraints[2]', '<<FolienVorlage>>.Constraints.distAussen * 360 / 2 / pi / .Constraints.RadiusFrontAussen')]
So, the error is that the expression engine property references Constraints[2] instead of Constraints[1].
wmayer
Site Admin
Posts: 15966
Joined: Thu Feb 19, 2009 10:32 am

Re: Undo in Sketcher creates inconsistent constraints

Postby wmayer » Sat May 23, 2020 5:38 pm

When adding a log message to the Copy & Paste methods of PropertyExpressionEngine and PropertyConstraintList then the output will be after each step:

Delete constraint:
PropertyConstraintList::Copy
PropertyExpressionEngine::Copy
Undo:
PropertyConstraintList::Paste
PropertyConstraintList::Copy
PropertyExpressionEngine::Copy
PropertyExpressionEngine::Paste
Redo:
PropertyExpressionEngine::Paste
PropertyExpressionEngine::Copy
PropertyConstraintList::Paste
PropertyConstraintList::Copy
Undo:
PropertyExpressionEngine::Paste
PropertyExpressionEngine::Copy
PropertyConstraintList::Paste
PropertyConstraintList::Copy
The point is the different order of function calls of the two Undo steps.

For the first undo it restores the constraints which internally renames the identifier (App::ObjectIdentifier) of the expression from Constraint[0] to Constraint[1] and then the previous identifier is restored which was bound to Constraint[1].

For the second undo it first restores the identifier but then restores the previous constraints. Because this again renames the internal identifiers we overwrite the correct value (Constraint[1]) with an incorrect value (Constraint[2]).

In the Constraints panel of the sketch you can also see that the f(x) symbol moves from the second to the third constraint.
wmayer
Site Admin
Posts: 15966
Joined: Thu Feb 19, 2009 10:32 am

Re: Undo in Sketcher creates inconsistent constraints

Postby wmayer » Sat May 23, 2020 7:36 pm

git commit 8d821fe5f

On Undo/Redo I don't think the class PropertyConstraintList should trigger the signals signalConstraintsRemoved() or signalConstraintsRenamed() because the correct values for the PropertyExpressionEngine will be restored from the transaction system of the document.

For me the above files work without problems now.
chrisb
Posts: 25204
Joined: Tue Mar 17, 2015 9:14 am

Re: Undo in Sketcher creates inconsistent constraints

Postby chrisb » Sat May 23, 2020 8:47 pm

Thanks! I will test when it is available for Mac in the precompiled version. I am confident that the issue is solved.
abdullah
Posts: 3504
Joined: Sun May 04, 2014 3:16 pm

Re: Undo in Sketcher creates inconsistent constraints

Postby abdullah » Sun May 24, 2020 5:30 am

wmayer wrote:
Sat May 23, 2020 7:36 pm
git commit 8d821fe5f

On Undo/Redo I don't think the class PropertyConstraintList should trigger the signals signalConstraintsRemoved() or signalConstraintsRenamed() because the correct values for the PropertyExpressionEngine will be restored from the transaction system of the document.

For me the above files work without problems now.
A nice way to separate undo/redo from other transactions without touching anything outside PropertyConstraintList (the path I was going...).