Skip to content

Commit

Permalink
Merge pull request #3584 from gwicke/area_tweaks
Browse files Browse the repository at this point in the history
Path: Area fixes and robustness tweaks
  • Loading branch information
sliptonic committed Jun 19, 2020
2 parents 58046eb + 439d363 commit 1e05734
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 55 deletions.
59 changes: 18 additions & 41 deletions src/Mod/Path/App/Area.cpp
Expand Up @@ -335,60 +335,31 @@ int Area::addShape(CArea &area, const TopoDS_Shape &shape, const gp_Trsf *trsf,
static std::vector<gp_Pnt> discretize(const TopoDS_Edge &edge, double deflection) {
std::vector<gp_Pnt> ret;
BRepAdaptor_Curve curve(edge);
Standard_Real efirst,elast,first,last;
Standard_Real efirst,elast;
efirst = curve.FirstParameter();
elast = curve.LastParameter();
bool reversed = (edge.Orientation()==TopAbs_REVERSED);

// push the first point
ret.push_back(curve.Value(reversed?elast:efirst));

Handle(Geom_Curve) c = BRep_Tool::Curve(edge, first, last);
first = c->FirstParameter();
last = c->LastParameter();
if(efirst>elast) {
if(first<last)
std::swap(first,last);
}else if(first>last)
std::swap(first,last);

// NOTE: OCCT QuasiUniformDeflection has a bug cause it to return only
// partial points for some (BSpline) curve if we pass in the edge trimmed
// first and last parameters. Passing the original curve first and last
// parameters works fine. The following algorithm uses the original curve
// parameters, and skip those out of range. The algorithm shall work the
// same for any other discetization algorithm, althgouth it seems only
// QuasiUniformDeflection has this bug.

// NOTE: QuasiUniformDeflection has trouble with some B-Spline, see
// https://forum.freecadweb.org/viewtopic.php?f=15&t=42628
//
// GCPnts_QuasiUniformDeflection discretizer(curve, deflection, first, last);
//
GCPnts_UniformDeflection discretizer(curve, deflection, first, last);
GCPnts_UniformDeflection discretizer(curve, deflection, efirst, elast);
if (!discretizer.IsDone ())
Standard_Failure::Raise("Curve discretization failed");
if(discretizer.NbPoints () > 1) {
int nbPoints = discretizer.NbPoints ();
//strangely OCC discretizer points are one-based, not zero-based, why?
if(reversed) {
for (int i=nbPoints-1; i>=1; --i) {
auto param = discretizer.Parameter(i);
if(first<last) {
if(param<efirst || param>elast)
continue;
}else if(param>efirst || param<elast)
continue;
ret.push_back(discretizer.Value(i));
}
}else{
for (int i=2; i<=nbPoints; i++) {
auto param = discretizer.Parameter(i);
if(first<last) {
if(param<efirst || param>elast)
continue;
}else if(param>efirst || param<elast)
continue;
ret.push_back(discretizer.Value(i));
}
}
Expand Down Expand Up @@ -867,14 +838,13 @@ struct WireJoiner {
// This algorithm tries to find a set of closed wires that includes as many
// edges (added by calling add() ) as possible. One edge may be included
// in more than one closed wires if it connects to more than one edges.
int findClosedWires() {
int findClosedWires(double tol = Precision::Confusion()) {
#if (BOOST_VERSION < 105500)
throw Base::RuntimeError("Module must be built with boost version >= 1.55");
#else
// It seems OCC projector sometimes mess up the tolerance of edges
// which are supposed to be connected. So use a lesser precision
// below, and call makeCleanWire to fix the tolerance
const double tol = 1e-10;
// Note on tolerance: It seems OCC projector sometimes mess up the
// tolerance of edges which are supposed to be connected. So use a
// lesser precision below, and call makeCleanWire to fix the tolerance

std::vector<VertexInfo> adjacentList;
std::set<EdgeInfo*> edgesToVisit;
Expand Down Expand Up @@ -1188,7 +1158,7 @@ static int foreachSubshape(const TopoDS_Shape &shape,
}
if(openShapes.empty())
return res;

BRep_Builder builder;
TopoDS_Compound comp;
builder.MakeCompound(comp);
Expand Down Expand Up @@ -1261,7 +1231,9 @@ TopoDS_Shape Area::findPlane(const TopoDS_Shape &shape, gp_Trsf &trsf)
}

int Area::project(TopoDS_Shape &shape_out,
const TopoDS_Shape &shape_in, const AreaParams *params)
const TopoDS_Shape &shape_in,
const AreaParams *params,
const TopoDS_Shape *work_plane)
{
FC_TIME_INIT2(t,t1);
Handle_HLRBRep_Algo brep_hlr = NULL;
Expand Down Expand Up @@ -1312,7 +1284,8 @@ int Area::project(TopoDS_Shape &shape_out,
showShape(v.edge,"split");
}

int skips = joiner.findClosedWires();
double tolerance = params ? params->Tolerance : Precision::Confusion();
int skips = joiner.findClosedWires(tolerance);
FC_TIME_LOG(t1,"WireJoiner findClosedWires");

showShape(joiner.comp,"pre_project");
Expand All @@ -1328,6 +1301,9 @@ int Area::project(TopoDS_Shape &shape_out,
area.myParams.Fill = TopExp_Explorer(shape_in,TopAbs_FACE).More()?FillFace:FillNone;
area.myParams.Coplanar = CoplanarNone;
area.myProjecting = true;
if (work_plane) {
area.myWorkPlane = *work_plane;
}
area.add(joiner.comp, OperationUnion);
const TopoDS_Shape &shape = area.getShape();

Expand Down Expand Up @@ -1630,7 +1606,8 @@ std::list<Area::Shape> Area::getProjectedShapes(const gp_Trsf &trsf, bool invers
mySkippedShapes = 0;
for(auto &s : myShapes) {
TopoDS_Shape out;
int skipped = Area::project(out,s.shape.Moved(loc),&myParams);
int skipped =
Area::project(out, s.shape.Moved(loc), &myParams, &myWorkPlane);
if(skipped < 0) {
++mySkippedShapes;
continue;
Expand Down Expand Up @@ -3303,7 +3280,7 @@ void Area::toPath(Toolpath &path, const std::list<TopoDS_Shape> &shapes,
(p.*setter)(retraction);
addGCode(false,path,plast,p,"G0");
}


plast = p;
bool first = true;
Expand Down
30 changes: 16 additions & 14 deletions src/Mod/Path/App/Area.h
Expand Up @@ -87,7 +87,7 @@ struct PathExport CAreaConfig {
/** For saving current libarea settings */
PARAM_DECLARE(PARAM_FNAME,AREA_PARAMS_CAREA)

/** The constructor automatically saves current setting and apply user defined ones
/** The constructor automatically saves current setting and apply user defined ones
*
* \arg \c p user defined configurations
* \arg \c noFitArgs if true, will override and disable arc fitting. Because
Expand Down Expand Up @@ -176,13 +176,13 @@ class PathExport Area: public Base::BaseClass {

bool isBuilt() const;

/** Set a working plane
/** Set a working plane
*
* \arg \c shape: a shape defining a working plane.
*
* The supplied shape does not need to be planar. Area will try to find planar
* sub-shape (face, wire or edge). If more than one planar sub-shape is found,
* it will prefer the top plane parallel to XY0 plane.
* sub-shape (face, wire or edge). If more than one planar sub-shape is found,
* it will prefer the top plane parallel to XY0 plane.
*
* If no working plane are set, Area will try to find a working plane from
* the added children shape using the same algorithm
Expand All @@ -199,7 +199,7 @@ class PathExport Area: public Base::BaseClass {
*/
TopoDS_Shape getPlane(gp_Trsf *trsf=0);

/** Add a child shape with given operation code
/** Add a child shape with given operation code
*
* No validation is done at this point. Exception will be thrown when asking
* for output shape, if any of the children shapes is not valid or not
Expand All @@ -217,7 +217,7 @@ class PathExport Area: public Base::BaseClass {
* If more than one offset is requested, a compound shape is return
* containing all offset shapes as wires regardless of \c Fill setting.
*/
TopoDS_Shape makeOffset(int index=-1, PARAM_ARGS_DEF(PARAM_FARG,AREA_PARAMS_OFFSET),
TopoDS_Shape makeOffset(int index=-1, PARAM_ARGS_DEF(PARAM_FARG,AREA_PARAMS_OFFSET),
int reoirent=0, bool from_center=false);

/** Make a pocket of the combined shape
Expand Down Expand Up @@ -277,7 +277,7 @@ class PathExport Area: public Base::BaseClass {
return mySections.size();
}

/** Add a OCC wire shape to CArea
/** Add a OCC wire shape to CArea
*
* \arg \c area: output converted curved object to here
* \arg \c wire: input wire object
Expand All @@ -287,10 +287,10 @@ class PathExport Area: public Base::BaseClass {
* \arg \c to_edges: if true, discretize all curves, and insert as open
* line segments
* */
static void addWire(CArea &area, const TopoDS_Wire &wire, const gp_Trsf *trsf=NULL,
double deflection=0.01, bool to_edges=false);
static void addWire(CArea &area, const TopoDS_Wire &wire, const gp_Trsf *trsf=NULL,
double deflection=0.01, bool to_edges=false);

/** Add a OCC generic shape to CArea
/** Add a OCC generic shape to CArea
*
* \arg \c area: output converted curved object to here
* \arg \c shape: input shape object
Expand All @@ -309,7 +309,7 @@ class PathExport Area: public Base::BaseClass {
* */
static int addShape(CArea &area, const TopoDS_Shape &shape, const gp_Trsf *trsf=NULL,
double deflection=0.01,const TopoDS_Shape *plane = NULL,
bool force_coplanar=true, CArea *areaOpen=NULL, bool to_edges=false,
bool force_coplanar=true, CArea *areaOpen=NULL, bool to_edges=false,
bool reorient=true);

/** Convert curves in CArea into an OCC shape
Expand All @@ -319,7 +319,7 @@ class PathExport Area: public Base::BaseClass {
* \arg \c trsf: optional transform matrix to transform the shape back into
* its original position.
* */
static TopoDS_Shape toShape(const CArea &area, bool fill,
static TopoDS_Shape toShape(const CArea &area, bool fill,
const gp_Trsf *trsf=NULL, int reoirent=0);

/** Convert a single curve into an OCC wire
Expand Down Expand Up @@ -363,14 +363,16 @@ class PathExport Area: public Base::BaseClass {
* \arg \c shapes: input list of shapes
* \arg \c pstart: output start point,
* \arg \c pend: optional output containing the ending point of the returned
*
*
* See #AREA_PARAMS_PATH for other arguments
*/
static void toPath(Toolpath &path, const std::list<TopoDS_Shape> &shapes,
const gp_Pnt *pstart=NULL, gp_Pnt *pend=NULL,
PARAM_ARGS_DEF(PARAM_FARG,AREA_PARAMS_PATH));

static int project(TopoDS_Shape &out, const TopoDS_Shape &in, const AreaParams *params=0);
static int project(TopoDS_Shape &out, const TopoDS_Shape &in,
const AreaParams *params = nullptr,
const TopoDS_Shape *work_plane = nullptr);

static void setWireOrientation(TopoDS_Wire& wire, const gp_Dir &dir, bool ccw);

Expand Down

0 comments on commit 1e05734

Please sign in to comment.