Skip to content

Commit

Permalink
[Sketcher] Refactor SketchObject::trim()
Browse files Browse the repository at this point in the history 
1. Use `Part::GeomCurve::createArc()`
2. Refactor constraint logic in `trim`
  • Loading branch information
@AjinkyaDahale
AjinkyaDahale committed Sep 29, 2024
1 parent c0f23c3 commit a89b85f
Showing 1 changed file with 88 additions and 168 deletions.
256 changes: 88 additions & 168 deletions src/Mod/Sketcher/App/SketchObject.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3394,18 +3394,18 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point)
return ((point1 - point2).Length() < 500 * Precision::Confusion()) ;
};

auto isPointAtPosition =
[this, &arePointsWithinPrecision](int GeoId1, PointPos pos1, Base::Vector3d point) {
Base::Vector3d pp = getPoint(GeoId1, pos1);
auto isPointAtPosition = [this, &arePointsWithinPrecision]
(int GeoId1, PointPos pos1, Base::Vector3d point) {
Base::Vector3d pp = getPoint(GeoId1, pos1);

return arePointsWithinPrecision(point, pp);
};
return arePointsWithinPrecision(point, pp);
};

// Checks whether preexisting constraints must be converted to new constraints.
// Preexisting point on object constraints get converted to coincidents, unless an end-to-end
// tangency is more relevant. returns by reference:
// - The type of constraint that should be used to constraint GeoId1 and GeoId
// - The element of GeoId1 to which the constraint should be applied.
// - The type of constraint that should be used to constraint GeoId1 and GeoId
// - The element of GeoId1 to which the constraint should be applied.
auto transformPreexistingConstraint = [this, isPointAtPosition](int GeoId,
int GeoId1,
Base::Vector3d point1,
Expand Down Expand Up @@ -3477,11 +3477,9 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point)
int GeoId1 = GeoEnum::GeoUndef, GeoId2 = GeoEnum::GeoUndef;
Base::Vector3d point1, point2;

// Remove B-splines' internal geometry beforehand so GeoId1 and GeoId2 do not change
if (geo->is<Part::GeomBSplineCurve>()) {
deleteUnusedInternalGeometryAndUpdateGeoId(GeoId);
geo = getGeometry(GeoId);
}
// Remove internal geometry beforehand so GeoId1 and GeoId2 do not change
deleteUnusedInternalGeometryAndUpdateGeoId(GeoId);
geo = getGeometry(GeoId);

// Using SketchObject wrapper, as Part2DObject version returns GeoId = -1 when intersection not
// found, which is wrong for a GeoId (axis). seekTrimPoints returns:
Expand Down Expand Up @@ -3510,10 +3508,10 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point)
double lastParam = static_cast<const Part::GeomCurve*>(geo)->getLastParameter();
double pointParam, point1Param, point2Param;
if (!getIntersectionParameters(
geo, point, pointParam, point1, point1Param, point2, point2Param))
geo, point, pointParam, point1, point1Param, point2, point2Param)) {
return -1;
}

bool ok = false;
bool startPointRemains = false;
bool endPointRemains = false;
std::vector<std::pair<double, double> > paramsOfNewGeos;
Expand All @@ -3522,121 +3520,29 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point)
std::vector<const Part::Geometry*> newGeosAsConsts;
bool oldGeoIsConstruction = GeometryFacade::getConstruction(static_cast<const Part::GeomCurve*>(geo));

auto setUpNewGeoLimitsFromNonPeriodic =
[&]() {
if (GeoId1 != GeoEnum::GeoUndef) {
startPointRemains = true;
paramsOfNewGeos.emplace_back(firstParam, point1Param);
}
if (GeoId2 != GeoEnum::GeoUndef) {
endPointRemains = true;
paramsOfNewGeos.emplace_back(point2Param, lastParam);
}
};

auto setUpNewGeoLimitsFromPeriodic =
[&]() {
startPointRemains = false;
endPointRemains = false;
if (GeoId1 != GeoEnum::GeoUndef && GeoId2 != GeoEnum::GeoUndef) {
paramsOfNewGeos.emplace_back(point2Param, point1Param);
}
};

auto createArcGeos = [&](const Part::GeomTrimmedCurve* curve) {
for (auto& [u1, u2] : paramsOfNewGeos) {
auto newArc = std::unique_ptr<Part::GeomTrimmedCurve>(
static_cast<Part::GeomTrimmedCurve*>(curve->copy()));
newArc->setRange(u1, u2);
newGeos.push_back(newArc.release());
if (isClosedCurve(geo)) {
startPointRemains = false;
endPointRemains = false;
if (GeoId1 != GeoEnum::GeoUndef && GeoId2 != GeoEnum::GeoUndef) {
paramsOfNewGeos.emplace_back(point2Param, point1Param);
}

return true;
};

auto createArcGeosForCircle = [&](const Part::GeomCircle* curve) {
for (auto& [u1, u2] : paramsOfNewGeos) {
auto newArc = std::make_unique<Part::GeomArcOfCircle>(
Handle(Geom_Circle)::DownCast(curve->handle()->Copy()));
newArc->setRange(u1, u2, false);
newGeos.push_back(newArc.release());
// int newId(GeoEnum::GeoUndef);
// newId = addGeometry(std::move(newArc));
// if (newId < 0) {
// return false;
// }
// newIds.push_back(newId);
// setConstruction(newId, GeometryFacade::getConstruction(curve));
// exposeInternalGeometry(newId);
}

return true;
};

auto createArcGeosForEllipse = [&](const Part::GeomEllipse* curve) {
for (auto& [u1, u2] : paramsOfNewGeos) {
auto newArc = std::make_unique<Part::GeomArcOfEllipse>(
Handle(Geom_Ellipse)::DownCast(curve->handle()->Copy()));
newArc->setRange(u1, u2, false);
newGeos.push_back(newArc.release());
}

return true;
};

auto createArcGeosForBSplineCurve = [&](const Part::GeomBSplineCurve* curve) {
for (auto& [u1, u2] : paramsOfNewGeos) {
auto newBsp = std::unique_ptr<Part::GeomBSplineCurve>(
static_cast<Part::GeomBSplineCurve*>(curve->copy()));
newBsp->Trim(u1, u2);
newGeos.push_back(newBsp.release());
}

return true;
};

if (geo->is<Part::GeomLineSegment>()) {
setUpNewGeoLimitsFromNonPeriodic();

ok = createArcGeos(static_cast<const Part::GeomTrimmedCurve*>(geo));
}
else if (geo->is<Part::GeomCircle>()) {
setUpNewGeoLimitsFromPeriodic();

ok = createArcGeosForCircle(static_cast<const Part::GeomCircle*>(geo));
}
else if (geo->is<Part::GeomEllipse>()) {
setUpNewGeoLimitsFromPeriodic();

ok = createArcGeosForEllipse(static_cast<const Part::GeomEllipse*>(geo));
}
else if (geo->is<Part::GeomArcOfCircle>()) {
setUpNewGeoLimitsFromNonPeriodic();

ok = createArcGeos(static_cast<const Part::GeomTrimmedCurve*>(geo));
}
else if (geo->isDerivedFrom<Part::GeomArcOfConic>()) {
setUpNewGeoLimitsFromNonPeriodic();

ok = createArcGeos(static_cast<const Part::GeomTrimmedCurve*>(geo));
}
else if (geo->is<Part::GeomBSplineCurve>()) {
const Part::GeomBSplineCurve* bsp = static_cast<const Part::GeomBSplineCurve*>(geo);

// what to do for periodic b-splines?
if (bsp->isPeriodic()) {
setUpNewGeoLimitsFromPeriodic();
else {
if (GeoId1 != GeoEnum::GeoUndef) {
startPointRemains = true;
paramsOfNewGeos.emplace_back(firstParam, point1Param);
}
else {
setUpNewGeoLimitsFromNonPeriodic();
if (GeoId2 != GeoEnum::GeoUndef) {
endPointRemains = true;
paramsOfNewGeos.emplace_back(point2Param, lastParam);
}

ok = createArcGeosForBSplineCurve(bsp);
}

if (!ok) {
delGeometries(newIds);
return -1;
for (auto& [u1, u2] : paramsOfNewGeos) {
auto newGeo = static_cast<const Part::GeomCurve*>(geo)->createArc(u1, u2);
assert(newGeo);
newGeos.push_back(newGeo);
newGeosAsConsts.push_back(newGeo);
}

switch (newGeos.size()) {
Expand All @@ -3661,16 +3567,15 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point)
}
}

for (auto& newGeo : newGeos) {
newGeosAsConsts.push_back(newGeo);
}

// Now that we have the new curves, change constraints as needed
// Some are covered with `deriveConstraintsForPieces`, others are specific to trim
// FIXME: We are using non-smart pointers since that's what's needed in `addConstraints`.
std::vector<Constraint*> newConstraints;

// A geoId we expect to never exist during this operation. We use it because newIds.front() is supposed to be `GeoId`, but we will delete constraints on it down the line, even if we will need it later. As a result, note that this will cause some malformed constraints until they are transferred back.
// A geoId we expect to never exist during this operation (but still not `GeoEnum::GeoUndef`).
// We use it because newIds.front() is supposed to be `GeoId`, but we will delete constraints
// on it down the line, even if we may need it later.
// Note that this will cause some malformed constraints until they are transferred back.
int nonexistentGeoId = getHighestCurveIndex() + newIds.size();
newIds.front() = nonexistentGeoId;

Expand Down Expand Up @@ -3708,79 +3613,94 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point)

const auto& allConstraints = this->Constraints.getValues();

bool isGeoId1CoincidentOnPoint1 = false;
bool isGeoId2CoincidentOnPoint2 = false;
bool isPoint1ConstrainedOnGeoId1 = false;
bool isPoint2ConstrainedOnGeoId2 = false;

// Workaround to avoid https://github.com/FreeCAD/FreeCAD/issues/15484.
// We will delete its internal geometry first and then replace GeoId with one of the curves.
// Of course, this will change `newIds`, and that's why we offset them.
std::vector<int> geoIdsToBeDeleted;
std::set< int, std::greater<> > geoIdsToBeDeleted;
// geoIdsToBeDeleted.push_back(GeoId);
if (hasInternalGeometry(geo)) {
for (const auto& oldConstrId: idsOfOldConstraints) {
if (allConstraints[oldConstrId]->Type == InternalAlignment) {
geoIdsToBeDeleted.push_back(allConstraints[oldConstrId]->First);
}
}

// NOTE: Assuming no duplication here.
// If there are redundants for some pathological reason, use std::unique.
std::sort(geoIdsToBeDeleted.begin(), geoIdsToBeDeleted.end(), std::greater<>());

// keep constraints on internal geometries so they are deleted
// when the old curve is deleted
}

for (const auto& oldConstrId: idsOfOldConstraints) {
// trim-specific changes first
const Constraint* con = allConstraints[oldConstrId];
bool newConstraintCreated = deriveConstraintsForPieces(GeoId, newIds, newGeosAsConsts, con, newConstraints);
// trim-specific changes once general changes are done
bool constraintWasModified = false;
switch (con->Type) {
case PointOnObject: {
if (!newConstraintCreated) {
break;
}
Constraint* newConstr = newConstraints.back();
// we might want to transform this (and the new point-on-object constraints) into a coincidence
if (newConstr->Second == newIds.front() &&
isPointAtPosition(newConstr->First, newConstr->FirstPos, point1)) {
if (con->First == GeoId1 && con->Second == GeoId
&& isPointAtPosition(con->First, con->FirstPos, point1)) {
// This concerns the start portion of the trim
Constraint* newConstr = con->copy();
newConstr->Type = Sketcher::Coincident;
newConstr->Second = newIds.front();
newConstr->SecondPos = PointPos::end;
if (newConstr->First == GeoId1) {
isGeoId1CoincidentOnPoint1 = true;
}
newConstraints.push_back(newConstr);
isPoint1ConstrainedOnGeoId1 = true;
constraintWasModified = true;
}
if (newConstr->Second == newIds.back() &&
isPointAtPosition(newConstr->First, newConstr->FirstPos, point2)) {
else if (con->First == GeoId2 && con->Second == GeoId
&& isPointAtPosition(con->First, con->FirstPos, point2)) {
// This concerns the end portion of the trim
Constraint* newConstr = con->copy();
newConstr->Type = Sketcher::Coincident;
newConstr->Second = newIds.back();
newConstr->SecondPos = PointPos::start;
if (newConstr->First == GeoId2) {
isGeoId2CoincidentOnPoint2 = true;
}
newConstraints.push_back(newConstr);
isPoint2ConstrainedOnGeoId2 = true;
constraintWasModified = true;
}
break;
}
case Tangent:
case Perpendicular: {
// TODO Tangent may have to be turned into endpoint-to-endpoint
// we might want to transform this into a coincidence
if (!newConstraintCreated) {
break;
// These may have to be turned into endpoint-to-endpoint or endpoint-to-edge
// TODO: could there be tangent/perpendicular constraints not involving the trim that are modified below?
if (con->involvesGeoId(GeoId1) && con->involvesGeoIdAndPosId(GeoId, PointPos::none)) {
Constraint* newConstr = con->copy();
newConstr->substituteIndexAndPos(GeoId, PointPos::none, newIds.front(), PointPos::end);
newConstraints.push_back(newConstr);
isPoint1ConstrainedOnGeoId1 = true;
constraintWasModified = true;
}
if (con->involvesGeoId(GeoId2) && con->involvesGeoIdAndPosId(GeoId, PointPos::none)) {
Constraint* newConstr = con->copy();
newConstr->substituteIndexAndPos(GeoId, PointPos::none, newIds.back(), PointPos::start);
newConstraints.push_back(newConstr);
isPoint2ConstrainedOnGeoId2 = true;
constraintWasModified = true;
}
if (constraintWasModified) {
// make sure the first position is a point
if (newConstraints.back()->FirstPos == PointPos::none) {
std::swap(newConstraints.back()->First, newConstraints.back()->Second);
std::swap(newConstraints.back()->FirstPos, newConstraints.back()->SecondPos);
}
// there is no need for the third point if it exists
newConstraints.back()->Third = GeoEnum::GeoUndef;
newConstraints.back()->ThirdPos = PointPos::none;
}
break;
}
case InternalAlignment: {
geoIdsToBeDeleted.insert(con->First);
break;
}
default:
break;
}
if (!constraintWasModified) {
deriveConstraintsForPieces(GeoId, newIds, newGeosAsConsts, con, newConstraints);
}
}

// Add point-on-object/coincidence constraints with the newly exposed points.
// This will need to account for the constraints that were already converted to coincident or end-to-end tangency/perpendicularity.
// This will need to account for the constraints that were already converted
// to coincident or end-to-end tangency/perpendicularity.
// TODO: Tangent/perpendicular not yet covered

if (GeoId1 != GeoEnum::GeoUndef && !isGeoId1CoincidentOnPoint1) {
if (GeoId1 != GeoEnum::GeoUndef && !isPoint1ConstrainedOnGeoId1) {
Constraint* newConstr = new Constraint();
newConstr->First = newIds.front();
newConstr->FirstPos = PointPos::end;
Expand All @@ -3801,7 +3721,7 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point)
newConstraints.push_back(newConstr);
}

if (GeoId2 != GeoEnum::GeoUndef && !isGeoId2CoincidentOnPoint2) {
if (GeoId2 != GeoEnum::GeoUndef && !isPoint2ConstrainedOnGeoId2) {
Constraint* newConstr = new Constraint();
newConstr->First = newIds.back();
newConstr->FirstPos = PointPos::start;
Expand Down

0 comments on commit a89b85f

Please sign in to comment.