From fa0fdf22cf6111e2273abd78a84cea8d867c9fd2 Mon Sep 17 00:00:00 2001 From: PaddleStroke Date: Thu, 27 Jun 2024 09:52:35 +0200 Subject: [PATCH 1/2] Sketcher: Dimension: Fix constraint switching. Refactor a little bit by adding finishDimensionCreation to reduce code copies. --- src/Mod/Sketcher/Gui/CommandConstraints.cpp | 104 +++++++------------- 1 file changed, 37 insertions(+), 67 deletions(-) diff --git a/src/Mod/Sketcher/Gui/CommandConstraints.cpp b/src/Mod/Sketcher/Gui/CommandConstraints.cpp index 1f05e07ebb..ca68d4e429 100644 --- a/src/Mod/Sketcher/Gui/CommandConstraints.cpp +++ b/src/Mod/Sketcher/Gui/CommandConstraints.cpp @@ -2183,7 +2183,6 @@ protected: specialConstraint = SpecialConstraint::LineOr2PointsDistance; } - bool arebothpointsorsegmentsfixed = isPointOrSegmentFixed(Obj, GeoId1) && isPointOrSegmentFixed(Obj, GeoId2); // Point-line case and point-circle/arc if (PosId1 != Sketcher::PointPos::none && PosId2 == Sketcher::PointPos::none) { Base::Vector3d pnt = Obj->getPoint(GeoId1, PosId1); @@ -2296,15 +2295,7 @@ protected: (pnt2 - pnt1).Length()); } - const std::vector& ConStr = Obj->Constraints.getValues(); - if (arebothpointsorsegmentsfixed || GeoId1 <= Sketcher::GeoEnum::RefExt - || constraintCreationMode == Reference) { - // it is a constraint on a external line, make it non-driving - Gui::cmdAppObjectArgs(Obj, "setDriving(%i,%s)", ConStr.size() - 1, "False"); - } - - numberOfConstraintsCreated++; - moveConstraint(ConStr.size() - 1, onSketchPos); + finishDimensionCreation(GeoId1, GeoId2, onSketchPos); } void createDistanceXYConstrain(Sketcher::ConstraintType type, int GeoId1, Sketcher::PointPos PosId1, int GeoId2, Sketcher::PointPos PosId2, Base::Vector2d onSketchPos) { @@ -2333,15 +2324,7 @@ protected: GeoId1, static_cast(PosId1), GeoId2, static_cast(PosId2), ActLength); } - const std::vector& ConStr = Obj->Constraints.getValues(); - if (areBothPointsOrSegmentsFixed(Obj, GeoId1, GeoId2) || constraintCreationMode == Reference) { - // it is a constraint on a external line, make it non-driving - Gui::cmdAppObjectArgs(Obj, "setDriving(%i,%s)", - ConStr.size() - 1, "False"); - } - - numberOfConstraintsCreated++; - moveConstraint(ConStr.size() - 1, onSketchPos); + finishDimensionCreation(GeoId1, GeoId2, onSketchPos); } void createRadiusDiameterConstrain(int GeoId, Base::Vector2d onSketchPos, bool firstCstr) { @@ -2385,15 +2368,7 @@ protected: } } - const std::vector& ConStr = Obj->Constraints.getValues(); - bool fixed = isPointOrSegmentFixed(Obj, GeoId); - if (fixed || constraintCreationMode == Reference || GeoId <= Sketcher::GeoEnum::RefExt) { - Gui::cmdAppObjectArgs(Obj, "setDriving(%i,%s)", ConStr.size() - 1, "False"); - } - - moveConstraint(ConStr.size() - 1, onSketchPos); - - numberOfConstraintsCreated ++; + finishDimensionCreation(GeoId, GeoEnum::GeoUndef, onSketchPos); } bool createCoincidenceConstrain(int GeoId1, Sketcher::PointPos PosId1, int GeoId2, Sketcher::PointPos PosId2) { @@ -2450,7 +2425,6 @@ protected: return; } - if (ActAngle == 0.0) { //Here we are sure that GeoIds are lines. So 0.0 means that lines are parallel, we change to distance restartCommand(QT_TRANSLATE_NOOP("Command", "Add Distance constraint")); @@ -2461,54 +2435,37 @@ protected: Gui::cmdAppObjectArgs(Obj, "addConstraint(Sketcher.Constraint('Angle',%d,%d,%d,%d,%f)) ", GeoId1, static_cast(PosId1), GeoId2, static_cast(PosId2), ActAngle); - const std::vector& ConStr = Obj->Constraints.getValues(); - if (areBothPointsOrSegmentsFixed(Obj, GeoId1, GeoId2) || constraintCreationMode == Reference) { - // it is a constraint on a external line, make it non-driving - - Gui::cmdAppObjectArgs(Obj, "setDriving(%i,%s)", ConStr.size() - 1, "False"); - } - numberOfConstraintsCreated++; - moveConstraint(ConStr.size() - 1, onSketchPos); + finishDimensionCreation(GeoId1, GeoId2, onSketchPos); } void createArcLengthConstrain(int GeoId, Base::Vector2d onSketchPos) { const Part::Geometry* geom = Obj->getGeometry(GeoId); - if (isArcOfCircle(*geom)) { - - const auto* arc = static_cast(geom); - double ActLength = arc->getAngle(false) * arc->getRadius(); - - Gui::cmdAppObjectArgs(Obj, "addConstraint(Sketcher.Constraint('Distance',%d,%f))", - GeoId, ActLength); - - const std::vector& ConStr = Obj->Constraints.getValues(); - if (isPointOrSegmentFixed(Obj, GeoId) || constraintCreationMode == Reference) { - // it is a constraint on a external line, make it non-driving - Gui::cmdAppObjectArgs(Obj, "setDriving(%i,%s)", ConStr.size() - 1, "False"); - } - numberOfConstraintsCreated++; - moveConstraint(ConStr.size() - 1, onSketchPos); + if (!isArcOfCircle(*geom)) { + return; } + + const auto* arc = static_cast(geom); + double ActLength = arc->getAngle(false) * arc->getRadius(); + + Gui::cmdAppObjectArgs(Obj, "addConstraint(Sketcher.Constraint('Distance',%d,%f))", + GeoId, ActLength); + + finishDimensionCreation(GeoId, GeoEnum::GeoUndef, onSketchPos); } void createArcAngleConstrain(int GeoId, Base::Vector2d onSketchPos) { const Part::Geometry* geom = Obj->getGeometry(GeoId); - if (isArcOfCircle(*geom)) { - - const auto* arc = static_cast(geom); - double angle = arc->getAngle(/*EmulateCCWXY=*/true); - - Gui::cmdAppObjectArgs(Obj, "addConstraint(Sketcher.Constraint('Angle',%d,%f))", - GeoId, angle); - - const std::vector& ConStr = Obj->Constraints.getValues(); - if (isPointOrSegmentFixed(Obj, GeoId) || constraintCreationMode == Reference) { - // it is a constraint on a external line, make it non-driving - Gui::cmdAppObjectArgs(Obj, "setDriving(%i,%s)", ConStr.size() - 1, "False"); - } - numberOfConstraintsCreated++; - moveConstraint(ConStr.size() - 1, onSketchPos); + if (!isArcOfCircle(*geom)) { + return; } + + const auto* arc = static_cast(geom); + double angle = arc->getAngle(/*EmulateCCWXY=*/true); + + Gui::cmdAppObjectArgs(Obj, "addConstraint(Sketcher.Constraint('Angle',%d,%f))", + GeoId, angle); + + finishDimensionCreation(GeoId, GeoEnum::GeoUndef, onSketchPos); } void createVerticalConstrain(int GeoId1, Sketcher::PointPos PosId1, int GeoId2, Sketcher::PointPos PosId2) { @@ -2708,6 +2665,19 @@ protected: return false; } + void finishDimensionCreation(int GeoId1, int GeoId2, Base::Vector2d onSketchPos) + { + bool fixed = GeoId2 == GeoEnum::GeoUndef ? isPointOrSegmentFixed(Obj, GeoId1) : areBothPointsOrSegmentsFixed(Obj, GeoId1, GeoId2); + + int index = Obj->Constraints.getValues().size() - 1; + if (fixed || constraintCreationMode == Reference) { + Gui::cmdAppObjectArgs(Obj, "setDriving(%i,%s)", index, "False"); + } + + numberOfConstraintsCreated++; + moveConstraint(index, onSketchPos); + } + void restartCommand(const char* cstrName) { specialConstraint = SpecialConstraint::None; Gui::Command::abortCommand(); From d92742f9c6caf9dc15dafad8a9b294ca04d61ae7 Mon Sep 17 00:00:00 2001 From: PaddleStroke Date: Tue, 2 Jul 2024 12:34:19 +0200 Subject: [PATCH 2/2] Sketcher: Dimension: Prevent the use of Undo from crashing. --- src/Mod/Sketcher/Gui/CommandConstraints.cpp | 94 ++++++++++++++------- 1 file changed, 62 insertions(+), 32 deletions(-) diff --git a/src/Mod/Sketcher/Gui/CommandConstraints.cpp b/src/Mod/Sketcher/Gui/CommandConstraints.cpp index ca68d4e429..90689a756f 100644 --- a/src/Mod/Sketcher/Gui/CommandConstraints.cpp +++ b/src/Mod/Sketcher/Gui/CommandConstraints.cpp @@ -27,6 +27,8 @@ #include #endif +#include + #include #include #include @@ -1385,7 +1387,7 @@ public: , selEllipseAndCo({}) , selSplineAndCo({}) , initialSelection(std::move(SubNames)) - , numberOfConstraintsCreated(0) + , cstrIndexes({}) { } ~DrawSketchHandlerDimension() override @@ -1468,10 +1470,6 @@ public: } makeAppropriateConstraint(previousOnSketchPos); } - else if (key == SoKeyboardEvent::Z && (QApplication::keyboardModifiers() & Qt::ControlModifier)) { - // User trying to cancel with Ctrl-Z - sketchgui->purgeHandler(); - } else { DrawSketchHandler::registerPressedKey(pressed, key); } @@ -1479,7 +1477,11 @@ public: void mouseMove(Base::Vector2d onSketchPos) override { - const std::vector& ConStr = Obj->Constraints.getValues(); + if (hasBeenAborted()) { + resetTool(); + return; + } + previousOnSketchPos = onSketchPos; //Change distance constraint based on position of mouse. @@ -1487,19 +1489,20 @@ public: updateDistanceType(onSketchPos); //Move constraints - if (numberOfConstraintsCreated > 0) { + if (cstrIndexes.size() > 0) { bool oneMoved = false; - for (int i = 0; i < numberOfConstraintsCreated; i++) { - if (ConStr[ConStr.size() - 1 - i]->isDimensional()) { + const std::vector& ConStr = Obj->Constraints.getValues(); + for (int index : cstrIndexes) { + if (ConStr[index]->isDimensional()) { Base::Vector2d pointWhereToMove = onSketchPos; if (specialConstraint == SpecialConstraint::Block) { - if (i == 0) - pointWhereToMove.x = Obj->getPoint(selPoints[0].GeoId, selPoints[0].PosId).x; - else + if (index == ConStr.size() - 1) pointWhereToMove.y = Obj->getPoint(selPoints[0].GeoId, selPoints[0].PosId).y; + else + pointWhereToMove.x = Obj->getPoint(selPoints[0].GeoId, selPoints[0].PosId).x; } - moveConstraint(ConStr.size() - 1 - i, pointWhereToMove); + moveConstraint(index, pointWhereToMove); oneMoved = true; } } @@ -1620,7 +1623,7 @@ protected: std::vector initialSelection; - int numberOfConstraintsCreated; + std::vector cstrIndexes; Sketcher::SketchObject* Obj; @@ -1671,16 +1674,21 @@ protected: void finalizeCommand() { + if (hasBeenAborted()) { + resetTool(); + return; + } + // Ask for the value of datum constraints ParameterGrp::handle hGrp = App::GetApplication().GetParameterGroupByPath("User parameter:BaseApp/Preferences/Mod/Sketcher"); bool show = hGrp->GetBool("ShowDialogOnDistanceConstraint", true); const std::vector& ConStr = Obj->Constraints.getValues(); bool commandHandledInEditDatum = false; - for (int i = numberOfConstraintsCreated - 1; i >= 0; i--) { - if (show && ConStr[ConStr.size() - 1 - i]->isDimensional() && ConStr[ConStr.size() - 1 - i]->isDriving) { + for (int index : cstrIndexes | boost::adaptors::reversed) { + if (show && ConStr[index]->isDimensional() && ConStr[index]->isDriving) { commandHandledInEditDatum = true; - EditDatumDialog editDatumDialog(sketchgui, ConStr.size() - 1 - i); + EditDatumDialog editDatumDialog(sketchgui, index); editDatumDialog.exec(); if (!editDatumDialog.isSuccess()) { break; @@ -1694,12 +1702,7 @@ protected: // This code enables the continuous creation mode. bool continuousMode = hGrp->GetBool("ContinuousCreationMode", true); if (continuousMode) { - Gui::Selection().clearSelection(); - Gui::Command::openCommand(QT_TRANSLATE_NOOP("Command", "Dimension")); - numberOfConstraintsCreated = 0; - specialConstraint = SpecialConstraint::None; - previousOnSketchPos = Base::Vector2d(0.f, 0.f); - clearRefVectors(); + resetTool(); } else { sketchgui->purgeHandler(); // no code after this line, Handler get deleted in ViewProvider @@ -2383,7 +2386,7 @@ protected: Gui::cmdAppObjectArgs(sketchgui->getObject(), "addConstraint(Sketcher.Constraint('Coincident', %d, %d, %d, %d)) ", GeoId1, static_cast(PosId1), GeoId2, static_cast(PosId2)); - numberOfConstraintsCreated++; + addConstraintIndex(); return true; } return false; @@ -2413,7 +2416,7 @@ protected: Gui::cmdAppObjectArgs(Obj, "addConstraint(Sketcher.Constraint('Equal',%d,%d)) ", GeoId1, GeoId2); - numberOfConstraintsCreated++; + addConstraintIndex(); } void createAngleConstrain(int GeoId1, int GeoId2, Base::Vector2d onSketchPos) { @@ -2479,7 +2482,7 @@ protected: Gui::cmdAppObjectArgs(sketchgui->getObject(), "addConstraint(Sketcher.Constraint('Vertical',%d,%d,%d,%d)) " , GeoId1, static_cast(PosId1), GeoId2, static_cast(PosId2)); } - numberOfConstraintsCreated++; + addConstraintIndex(); tryAutoRecompute(Obj); } void createHorizontalConstrain(int GeoId1, Sketcher::PointPos PosId1, int GeoId2, Sketcher::PointPos PosId2) { @@ -2493,14 +2496,14 @@ protected: Gui::cmdAppObjectArgs(sketchgui->getObject(), "addConstraint(Sketcher.Constraint('Horizontal',%d,%d,%d,%d)) " , GeoId1, static_cast(PosId1), GeoId2, static_cast(PosId2)); } - numberOfConstraintsCreated++; + addConstraintIndex(); tryAutoRecompute(Obj); } void createBlockConstrain(int GeoId) { Gui::cmdAppObjectArgs(sketchgui->getObject(), "addConstraint(Sketcher.Constraint('Block',%d)) ", GeoId); - numberOfConstraintsCreated++; + addConstraintIndex(); tryAutoRecompute(Obj); } @@ -2545,7 +2548,7 @@ protected: Gui::cmdAppObjectArgs(Obj, "addConstraint(Sketcher.Constraint('Symmetric',%d,%d,%d,%d,%d)) ", GeoId1, static_cast(PosId1), GeoId2, static_cast(PosId2), GeoId3); - numberOfConstraintsCreated++; + addConstraintIndex(); tryAutoRecompute(Obj); } } @@ -2569,7 +2572,7 @@ protected: Gui::cmdAppObjectArgs(Obj, "addConstraint(Sketcher.Constraint('Symmetric',%d,%d,%d,%d,%d,%d)) ", GeoId1, static_cast(PosId1), GeoId2, static_cast(PosId2), GeoId3, static_cast(PosId3)); - numberOfConstraintsCreated++; + addConstraintIndex(); tryAutoRecompute(Obj); } } @@ -2674,10 +2677,27 @@ protected: Gui::cmdAppObjectArgs(Obj, "setDriving(%i,%s)", index, "False"); } - numberOfConstraintsCreated++; + addConstraintIndex(); moveConstraint(index, onSketchPos); } + void addConstraintIndex() + { + cstrIndexes.push_back(Obj->Constraints.getValues().size() - 1); + } + + bool hasBeenAborted() + { + // User can abort the command with Undo (ctrl-Z) + if (cstrIndexes.size() > 0) { + if (cstrIndexes.back() != Obj->Constraints.getValues().size() - 1 ) { + return true; + } + } + + return false; + } + void restartCommand(const char* cstrName) { specialConstraint = SpecialConstraint::None; Gui::Command::abortCommand(); @@ -2685,7 +2705,17 @@ protected: sketchgui->draw(false, false); // Redraw Gui::Command::openCommand(cstrName); - numberOfConstraintsCreated = 0; + cstrIndexes.clear(); + } + + void resetTool() + { + Gui::Selection().clearSelection(); + Gui::Command::openCommand(QT_TRANSLATE_NOOP("Command", "Dimension")); + cstrIndexes.clear(); + specialConstraint = SpecialConstraint::None; + previousOnSketchPos = Base::Vector2d(0.f, 0.f); + clearRefVectors(); } };