From 952aaa458e2b98dc28170f0167fa81c6e5cacbe7 Mon Sep 17 00:00:00 2001 From: Abdullah Tahiri Date: Sun, 31 Dec 2017 18:52:54 +0100 Subject: [PATCH] Sketcher: Fix transfer of tangency and perpendicular end-to-endpoint constraints ================================================================================ When transfering constraints to coincident points on deletion of geometry, a tangency/perpendicular constraint cannot be blindly transfered as the destination edge may not be tangent/perpendicular leading to unexpected behaviour. However, the user does expect that something that was coincident with such end-to-endpoint constraint (which implicitly includes a coincident constraint) remains coincident after deletion. Therefore, the change of type to coincident. This implicitly solves the problem of representation of constraints leading to a crash in coin3d. fixes #3291 --- src/Mod/Sketcher/App/SketchObject.cpp | 34 +++++++++++++++++++-------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/src/Mod/Sketcher/App/SketchObject.cpp b/src/Mod/Sketcher/App/SketchObject.cpp index 7e15171280..a011dbdf0a 100644 --- a/src/Mod/Sketcher/App/SketchObject.cpp +++ b/src/Mod/Sketcher/App/SketchObject.cpp @@ -1015,20 +1015,34 @@ int SketchObject::transferConstraints(int fromGeoId, PointPos fromPosId, int toG if (vals[i]->First == fromGeoId && vals[i]->FirstPos == fromPosId && !(vals[i]->Second == toGeoId && vals[i]->SecondPos == toPosId) && !(toGeoId < 0 && vals[i]->Second <0) ) { - Constraint *constNew = newVals[i]->clone(); - constNew->First = toGeoId; - constNew->FirstPos = toPosId; - newVals[i] = constNew; - changed.push_back(constNew); + // Nothing guarantees that a tangent can be freely transferred to another coincident point, as + // the transfer destination edge most likely won't be intended to be tangent. However, if it is + // an end to end point tangency, the user expects it to be substituted by a coincidence constraint. + Constraint *constNew = newVals[i]->clone(); + constNew->First = toGeoId; + constNew->FirstPos = toPosId; + + if(vals[i]->Type == Sketcher::Tangent || vals[i]->Type == Sketcher::Perpendicular) + constNew->Type = Sketcher::Coincident; + + newVals[i] = constNew; + changed.push_back(constNew); } else if (vals[i]->Second == fromGeoId && vals[i]->SecondPos == fromPosId && !(vals[i]->First == toGeoId && vals[i]->FirstPos == toPosId) && !(toGeoId < 0 && vals[i]->First< 0)) { - Constraint *constNew = newVals[i]->clone(); - constNew->Second = toGeoId; - constNew->SecondPos = toPosId; - newVals[i] = constNew; - changed.push_back(constNew); + + Constraint *constNew = newVals[i]->clone(); + constNew->Second = toGeoId; + constNew->SecondPos = toPosId; + // Nothing guarantees that a tangent can be freely transferred to another coincident point, as + // the transfer destination edge most likely won't be intended to be tangent. However, if it is + // an end to end point tangency, the user expects it to be substituted by a coincidence constraint. + if(vals[i]->Type == Sketcher::Tangent || vals[i]->Type == Sketcher::Perpendicular) + constNew->Type = Sketcher::Coincident; + + newVals[i] = constNew; + changed.push_back(constNew); } }