Skip to content

Commit

Permalink
[Sketcher] Fix endpoint-to-endpoint/edge tangency substitution
Browse files Browse the repository at this point in the history 
Only substitute if the point(s) involved are `start`/`end`. Centers do
not make sense here.
  • Loading branch information
@AjinkyaDahale
AjinkyaDahale committed Aug 20, 2024
1 parent 131956e commit e6a95d1
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 51 deletions.
22 changes: 17 additions & 5 deletions src/Mod/Sketcher/Gui/CommandConstraints.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3777,7 +3777,9 @@ bool CmdSketcherConstrainCoincidentUnified::substituteConstraintCombinationsPoin
if ((*it)->Type == Sketcher::Tangent && (*it)->FirstPos == Sketcher::PointPos::none
&& (*it)->SecondPos == Sketcher::PointPos::none && (*it)->Third == GeoEnum::GeoUndef
&& (((*it)->First == GeoId1 && (*it)->Second == GeoId2)
|| ((*it)->Second == GeoId1 && (*it)->First == GeoId2))) {
|| ((*it)->Second == GeoId1 && (*it)->First == GeoId2))
&& (PosId1 == Sketcher::PointPos::start
|| PosId1 == Sketcher::PointPos::end)) {

// NOTE: This function does not either open or commit a command as it is used for group
// addition it relies on such infrastructure being provided by the caller.
Expand Down Expand Up @@ -3813,6 +3815,12 @@ bool CmdSketcherConstrainCoincidentUnified::substituteConstraintCombinationsCoin
if ((*it)->Type == Sketcher::Tangent && (*it)->Third == GeoEnum::GeoUndef
&& (((*it)->First == GeoId1 && (*it)->Second == GeoId2)
|| ((*it)->Second == GeoId1 && (*it)->First == GeoId2))) {
if (!(PosId1 == Sketcher::PointPos::start
|| PosId1 == Sketcher::PointPos::end)
|| !(PosId2 == Sketcher::PointPos::start
|| PosId2 == Sketcher::PointPos::end)) {
continue;
}
if ((*it)->FirstPos == Sketcher::PointPos::none
&& (*it)->SecondPos == Sketcher::PointPos::none) {

Expand Down Expand Up @@ -6569,8 +6577,11 @@ bool CmdSketcherConstrainTangent::substituteConstraintCombinations(SketchObject*
++it, ++cid) {
if ((*it)->Type == Sketcher::Coincident
&& (((*it)->First == GeoId1 && (*it)->Second == GeoId2)
|| ((*it)->Second == GeoId1 && (*it)->First == GeoId2))) {

|| ((*it)->Second == GeoId1 && (*it)->First == GeoId2))
&& ((*it)->FirstPos == Sketcher::PointPos::start
|| (*it)->FirstPos == Sketcher::PointPos::end)
&& ((*it)->SecondPos == Sketcher::PointPos::start
|| (*it)->SecondPos == Sketcher::PointPos::end)) {
// save values because 'doEndpointTangency' changes the
// constraint property and thus invalidates this iterator
int first = (*it)->First;
Expand All @@ -6596,8 +6607,9 @@ bool CmdSketcherConstrainTangent::substituteConstraintCombinations(SketchObject*
}
else if ((*it)->Type == Sketcher::PointOnObject
&& (((*it)->First == GeoId1 && (*it)->Second == GeoId2)
|| ((*it)->Second == GeoId1 && (*it)->First == GeoId2))) {

|| ((*it)->Second == GeoId1 && (*it)->First == GeoId2))
&& ((*it)->FirstPos == Sketcher::PointPos::start
|| (*it)->FirstPos == Sketcher::PointPos::end)) {
Gui::Command::openCommand(
QT_TRANSLATE_NOOP("Command",
"Swap point on object and tangency with point to curve tangency"));
Expand Down
103 changes: 61 additions & 42 deletions src/Mod/Sketcher/Gui/DrawSketchDefaultHandler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -666,19 +666,26 @@ class DrawSketchDefaultHandler: public DrawSketchHandler,
}

// find if there is already a matching tangency
auto result = std::find_if(AutoConstraints.begin(),
AutoConstraints.end(),
[&](const auto& ace) {
return ace->Type == Sketcher::Tangent
&& ace->First == geoId1
&& ace->Second == ac.GeoId;
});


if (result
!= AutoConstraints.end()) { // modify tangency to endpoint-to-endpoint
(*result)->FirstPos = posId1;
(*result)->SecondPos = ac.PosId;
auto itOfTangentConstraint = AutoConstraints.end();
if ((posId1 == Sketcher::PointPos::start
|| posId1 == Sketcher::PointPos::end)
&& (ac.PosId == Sketcher::PointPos::start
|| ac.PosId == Sketcher::PointPos::end)) {
itOfTangentConstraint =
std::find_if(AutoConstraints.begin(),
AutoConstraints.end(),
[&](const auto& ace) {
return ace->Type == Sketcher::Tangent
&& ace->First == geoId1
&& ace->Second == ac.GeoId;
});
}


if (itOfTangentConstraint != AutoConstraints.end()) {
// modify tangency to endpoint-to-endpoint
(*itOfTangentConstraint)->FirstPos = posId1;
(*itOfTangentConstraint)->SecondPos = ac.PosId;
}
else {
auto c = std::make_unique<Sketcher::Constraint>();
Expand All @@ -699,21 +706,27 @@ class DrawSketchDefaultHandler: public DrawSketchHandler,
std::swap(posId1, posId2);
}

auto result = std::find_if(AutoConstraints.begin(),
AutoConstraints.end(),
[&](const auto& ace) {
return ace->Type == Sketcher::Tangent
&& ace->First == geoId1
&& ace->Second == ac.GeoId;
});
auto itOfTangentConstraint = AutoConstraints.end();
if (posId1 == Sketcher::PointPos::start
|| posId1 == Sketcher::PointPos::end) {
itOfTangentConstraint =
std::find_if(AutoConstraints.begin(),
AutoConstraints.end(),
[&](const auto& ace) {
return ace->Type == Sketcher::Tangent
&& ace->First == geoId1
&& ace->Second == ac.GeoId;
});
}

// if tangency, convert to point-to-edge tangency
if (result != AutoConstraints.end()) {
(*result)->FirstPos = posId1;

if ((*result)->First != geoId1) {
std::swap((*result)->Second, (*result)->First);
if (itOfTangentConstraint != AutoConstraints.end()) {
if ((*itOfTangentConstraint)->First != geoId1) {
std::swap((*itOfTangentConstraint)->Second,
(*itOfTangentConstraint)->First);
}

(*itOfTangentConstraint)->FirstPos = posId1;
}
else {
auto c = std::make_unique<Sketcher::Constraint>();
Expand All @@ -735,11 +748,11 @@ class DrawSketchDefaultHandler: public DrawSketchHandler,
c->ThirdPos = posId1;
AutoConstraints.push_back(std::move(c));
} break;
// In special case of Horizontal/Vertical constraint, geoId2 is normally unused
// and should be 'Constraint::GeoUndef' However it can be used as a way to
// require the function to apply these constraints on another geometry In this
// case the caller as to set geoId2, then it will be used as target instead of
// geoId2
// In special case of Horizontal/Vertical constraint, geoId2 is normally
// unused and should be 'Constraint::GeoUndef' However it can be used as a
// way to require the function to apply these constraints on another
// geometry In this case the caller as to set geoId2, then it will be used
// as target instead of geoId2
case Sketcher::Horizontal: {
auto c = std::make_unique<Sketcher::Constraint>();
c->Type = Sketcher::Horizontal;
Expand Down Expand Up @@ -777,9 +790,10 @@ class DrawSketchDefaultHandler: public DrawSketchHandler,
|| geom2->getTypeId() == Part::GeomCircle::getClassTypeId()
|| geom2->getTypeId() == Part::GeomArcOfCircle::getClassTypeId()) {
// in all these cases an intermediate element is needed
/*makeTangentToEllipseviaNewPoint(Obj,
static_cast<const Part::GeomEllipse
*>(geom1), geom2, geoId1, geoId2);*/
// makeTangentToEllipseviaNewPoint(
// Obj,
// static_cast<const Part::GeomEllipse *>(geom1),
// geom2, geoId1, geoId2);
// NOTE: Temporarily deactivated
return;
}
Expand All @@ -803,11 +817,10 @@ class DrawSketchDefaultHandler: public DrawSketchHandler,
|| geom2->getTypeId() == Part::GeomCircle::getClassTypeId()
|| geom2->getTypeId() == Part::GeomArcOfCircle::getClassTypeId()) {
// in all these cases an intermediate element is needed
// makeTangentToArcOfEllipseviaNewPoint(Obj,
// static_cast<const
// Part::GeomArcOfEllipse
// *>(geom1), geom2, geoId1,
// geoId2);
// makeTangentToArcOfEllipseviaNewPoint(
// Obj,
// static_cast<const Part::GeomArcOfEllipse*>(geom1), geom2,
// geoId1, geoId2);
// NOTE: Temporarily deactivated
return;
}
Expand All @@ -830,12 +843,18 @@ class DrawSketchDefaultHandler: public DrawSketchHandler,
|| (ace->First == ac.GeoId && ace->Second == geoId1));
});

if (resultcoincident
!= AutoConstraints.end()) { // endpoint-to-endpoint tangency
if (resultcoincident != AutoConstraints.end()
&& ((*resultcoincident)->FirstPos == Sketcher::PointPos::start
|| (*resultcoincident)->FirstPos == Sketcher::PointPos::end)
&& ((*resultcoincident)->SecondPos == Sketcher::PointPos::start
|| (*resultcoincident)->SecondPos == Sketcher::PointPos::end)) {
// endpoint-to-endpoint tangency
(*resultcoincident)->Type = Sketcher::Tangent;
}
else if (resultpointonobject
!= AutoConstraints.end()) { // endpoint-to-edge tangency
else if (resultpointonobject != AutoConstraints.end()
&& ((*resultcoincident)->FirstPos == Sketcher::PointPos::start
|| (*resultcoincident)->FirstPos == Sketcher::PointPos::end)) {
// endpoint-to-edge tangency
(*resultpointonobject)->Type = Sketcher::Tangent;
}
else { // regular edge to edge tangency
Expand Down
11 changes: 7 additions & 4 deletions src/Mod/Sketcher/Gui/DrawSketchHandler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -440,8 +440,8 @@ int DrawSketchHandler::seekAutoConstraint(std::vector<AutoConstraint>& suggested
int preSelPnt = getPreselectPoint();
int preSelCrv = getPreselectCurve();
int preSelCrs = getPreselectCross();
int GeoId = GeoEnum::GeoUndef;

int GeoId = GeoEnum::GeoUndef;
PointPos PosId = PointPos::none;

if (preSelPnt != -1) {
Expand All @@ -459,15 +459,18 @@ int DrawSketchHandler::seekAutoConstraint(std::vector<AutoConstraint>& suggested
}
}
}
else if (preSelCrs == 0) { // root point
else if (preSelCrs == 0) {
// root point
GeoId = Sketcher::GeoEnum::RtPnt;
PosId = PointPos::start;
}
else if (preSelCrs == 1) { // x axis
else if (preSelCrs == 1) {
// x axis
GeoId = Sketcher::GeoEnum::HAxis;
hitShapeDir = Base::Vector3d(1, 0, 0);
}
else if (preSelCrs == 2) { // y axis
else if (preSelCrs == 2) {
// y axis
GeoId = Sketcher::GeoEnum::VAxis;
hitShapeDir = Base::Vector3d(0, 1, 0);
}
Expand Down

0 comments on commit e6a95d1

Please sign in to comment.