Reorient sketch loses constraints

[chrisf]

The attached has a cube with a fully constrained sketch on one face. The sketch is a square (horizontal, vertical and coincident constraints) with two edges fixed length and the bottom left corner fixed. When I do PartDesign->Reorient Sketch (removing it from its support) to the x,y plane, the sketch loses all of the line constraints leaving just the bottom left corner position fixed. I get the same behaviour if I assign App.ActiveDocument.Sketch.Support=None.

I can't see a reason why the constraint should be removed. Can anyone confirm this behaviour?

Thanks
Chris

OS: Ubuntu 13.10
Platform: 64-bit
Version: 0.14.3398 (Git)
Branch: master
Hash: 95bda087643493a7e31e9cf507f1041fd446c4c7
Python version: 2.7.5+
Qt version: 4.8.4
Coin version: 4.0.0a
SoQt version: 1.5.0
OCC version: 6.5.4

[wmayer]

I can't see a reason why the constraint should be removed. Can anyone confirm this behaviour?

I can't see this in the code right now where it happens but I can confirm that it happens. However, it's always problematic to change the support face or move to a new support face because there it easily happens that the sketch gets into an inconsistent state. So, from this point of view it's a safety thing to clear unknown ids from the external geometry list.

[chrisf]

Hi Werner,
Thanks for looking at this. I understand the inconsistency issues you're referring to. I'm using these macros to work around those very effectively at the moment. Working on a fairly large model and I haven't seen any instability problems at all in three days of design work. The macros work fine for modifying a sketch within the model tree. In order to add a new sketch into the tree or delete a sketch from the tree, though, I need to orphan sketches temporarily before reinstating their support. The macros work well for reinstating the correct face and external geometry in this case but due to this issue I lose most of the constraints.

I'd happily look over the code if you could provide a pointer to the appropriate files in src/Mod/Sketcher?
Chris

[triplus]

While i do not see this modelling strategy sensible to be used ATM i agree the truncating part of existing constraints in sketch should not happen. I guess this was implemented because unpredicted results could be the outcome of this operation as wmayer suggested:

However, it's always problematic to change the support face or move to a new support face because there it easily happens that the sketch gets into an inconsistent state. So, from this point of view it's a safety thing to clear unknown ids from the external geometry list.

But for users doing it like this i guess they have to be aware there will be a lot of manual repairing involved in procedure but for that to happen it still makes sense to get the sketch in as little modified state as possible compared to original one. And to fix manually what broke down in the operation itself.

P.S. How to explain new users "broken sketch" is there for their convenience in the first place for scenarios where they do want to use modelling strategy not sensible to be used ATM is another thing. But then again i guess we manage.

[chrisf]

Are we sure that this is deliberate behaviour? Not withstanding the potential benefit of deleting links to external geometry, deleting horizontal/vertical and length constraints from lines doesn't appear something that could improve re-usability of a sketch on a new face.

I had thought that it would be occurring in the sketch edit code but it appears to be in some code triggered on a change of the Support member. This is on the fcstd file I posted earlier:

>>> App.ActiveDocument.Sketch.ConstraintCount
12
>>> App.ActiveDocument.Sketch.Support=None
>>> App.ActiveDocument.Sketch.ConstraintCount
6

[triplus]

I don't know if this is deliberate behaviour or not and that is something one of the devs will have to explain. Making some tests i see different behaviour when something (dimension/constraint) gets truncated and when not in the sketch. I would guess the decision is made based on what was said:

So, from this point of view it's a safety thing to clear unknown ids from the external geometry list.

I guess when sketch gets detached from external geometry face sketch elements like lines are affected by Topological Naming issue too? Sketcher just doesn't know where to put existing constraint/dimension for elements with ID changed in new detached sketch? Could this be it or sketches are less affected by this issues? If true one can not be 100% sure existing constraints/dimensions are correctly positioned because by "re-calculating ID" for each element on detach operation one of the elements can get ID that was used for different element in original sketch?

[chrisf]

It looks to me that there are some problems in the external geometry code.

I traced the cause as far as the SketchObject::onChanged code:

Code: Select all

void SketchObject::onChanged(const App::Property* prop)
...
    else if (prop == &Support) {
        // 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)
            }
        }

This code responds to changes in the SketchObject. If the Support has changed and Support.getValue() is zero (the sketch has been orphaned) it passes two empty vectors to ExternalGeometry.setValues presumably to clear the external geometry list.

Note that the external geometry list is only cleared if the support has changed to zero whereas, I would have thought that it should be cleared in any case that the support member changes, not only when it changes to zero.

It also appears that this resetting of the (App::PropertyLinkSubList) ExternalGeometry is having the knock on effect of breaking the constraints list. I've no idea how that's happening yet.

I have commented out this call to ExternalGeometry.setValues in my code and the constraints are preserved when orphaning a sketch.

Note that, as the code stands, you can remap a sketch to a different face and the external geometry list is not cleared. You can make a change in the tree that completely changes the edge/face numbering and the external geometry list is not cleared. It seems in this situation that there is little benefit in clearing the external geometry list in the SketchObject::onChanged code.

[chrisf]

This provides a stable delete of external geometry in the sketch if the support changes (whether it changes to '0' or something else) without losing unaffected constraints.
You'll see that I've got a call to delConstraintsToExternal() commented out. If this is included, the constraints are still lost.

Code: Select all

void SketchObject::onChanged(const App::Property* prop)
...
    else if (prop == &Support) {
        // make sure not to change anything while restoring this object
        if (!isRestoring()) {
            // if support face was cleared then also clear the external geometry
//            delConstraintsToExternal();
			for (int i=0; i < getExternalGeometryCount(); i++) {
				delExternal(0);
			}
        }
    }

[chrisf]

The delConstraintsToExternal code did not respect the '-2000' magic number for constraints that do not contain an object index. This fixes it and allows you to call delConstraintsToExternal in SketchObject::onChanged.

Code: Select all

int SketchObject::delConstraintsToExternal()
{
    const std::vector< Constraint * > &constraints = Constraints.getValues();
    std::vector< Constraint * > newConstraints(0);
    int GeoId = -3, NullId = -2000;
    for (std::vector<Constraint *>::const_iterator it = constraints.begin();
         it != constraints.end(); ++it) {
//        if ((*it)->First > GeoId && (*it)->Second > GeoId) {
//            newConstraints.push_back(*it);
//        }
         if ((*it)->First > GeoId && ((*it)->Second > GeoId || (*it)->Second == NullId) && ((*it)->Third > GeoId || (*it)->Third == NullId)) {
			 newConstraints.push_back(*it);
		 }
    }

    Constraints.setValues(newConstraints);
    Constraints.acceptGeometry(getCompleteGeometry());

    return 0;
}

[logari81]

If all constraints disappear (also those not involving external geometries) then this is a bug. They are not actually deleted, they are just hidden/disabled.

This was a quirk for solving some issues with undoing/redoing. As far as I remember, the constraints list in a geometry is aware of the geometry it applies. If the geometry is changed and we do inform the constraints list that the new geometry is valid, the Constraints list detects a mismatch to the last accepted geometry and hides all its content.

So if constraints disappears, this happens because somewhere in C++ we forgot to inform the constraints list that the new geometry after some operation, is still valid (if it is or fix it if it isn't).