Skip to content

Commit

Permalink
Sketcher: Major re-structuration: New Solving Model: General Sketch S…
Browse files Browse the repository at this point in the history
…olve call reduction

=======================================================================================

ActSketch in ViewProvider dissapears. The temporal sketch (sketch.cpp) for solving is now a data member of SketchObject.cpp (hereinafter solvedSketch). All the solving is concentrated in SketchObject.cpp.

SketchObject provides an interface to expose its solver, as it is still currently needed for some UI operations from ViewProviderSketch, like dragging points (solving rubber bands).

ViewProviderSketch still can select whether to draw the solvedSketch geometry (previously ActSketch) geometry (UI staff) or the SketchObject geometry. Performancewise, it makes sense to separate this two
geometries, as the SketchObject one involves modifying the Geometry and Constraint Properties (including all the undo related functionality), which would mess the undo functinality and incur in a big
peformance penalisation while dragging. One characteristic of solvedSketch is that its geometry is solved, whereas the geometry of SketchObject may not have been solved yet.

These geometries may differ at for the following reasons:
1. The geometry corresponds to an ongoing dragging operation, so solvedSketch has the last calculated dragging geometry, while SketchObject has the one corresponding to initial position.
2. Geometry/constraints were added to the sketch, but no solve/execute was triggered yet (envisioned situation is the future group creation functionality not in this commit).

What do I gain?

- Inserting a (simple) geometry element now costs 1 solver execution.
- Inserting a constraint now costs 1 solver executions.

For reference, in previous versions inserting a constraint involved 5 solver executions.

The following information provide a historical review of the coding of this commit (formed of 10 squashed commits):

This is a general sketch solve call reduction, not only during geometry creation (this commit does not include until here any specific measure to reduce calls on geometry creation, that is another branch)

After a lot of profiling, it seems that the "cause"(tm) that creates so many solver calls is that every update generates a solving in ViewProviderSketch, regardless of the need for that update,
many times with the only aim of providing the DoF for the message dialog and keeping ActSketch in sync with SketchObject in case it is needed (currently UI moving points, constraints,...).

Sketch solver is now a data member of SketchObject instead of a temporal object that gets initilized and destroyed.

This allows:
1. Potentially more synergy reducing calls to setUpSketch (still to be seen, still to be optimized)
2. Allowing SketchObject to access the latest geometry that has been solved => In future, allow objects that use SketchObject to obtain the latest
solved geometry instead the geometry of SketchObject that may still be unsolved. This is relevant for drawing the geometry

No more solving in ViewProviderSketch. Solving a Sketch is now an exclusive competence of SketchObject.

There is however a lot of cleaning to do in ViewProviderSketch

(I mean, not that these commits are making a mess in VPSketch,
but that as a consequence of the changes, it should be possible to
optimize VPSketch, specially moving and drawing methods)

Very useful comment for future developers that may wonder why a solve per constraint just upon addition is necessary.

Added a new function to get the size of the geometry of the instance of the solver object (Sketch.cpp).
The previous way was to extract the geometry, which is costly and error prone, as you have to delete it afterwards.

Inserted comment about the necessity of triggering a Part2D update during edit mode
  • Loading branch information
abdullahtahiriyo authored and wwmayer committed Jun 15, 2015
1 parent 280019d commit 0e92e63
Show file tree
Hide file tree
Showing 9 changed files with 254 additions and 139 deletions.
1 change: 1 addition & 0 deletions src/Mod/PartDesign/Gui/CMakeLists.txt
Expand Up @@ -15,6 +15,7 @@ include_directories(
${ZLIB_INCLUDE_DIR}
${PYTHON_INCLUDE_PATH}
${XERCESC_INCLUDE_DIR}
${EIGEN3_INCLUDE_DIR}
)
link_directories(${OCC_LIBRARY_DIR})

Expand Down
3 changes: 3 additions & 0 deletions src/Mod/Sketcher/App/Sketch.h
Expand Up @@ -315,6 +315,9 @@ class SketcherExport Sketch :public Base::Persistence
//see more info in respective function in GCS.
double calculateConstraintError(int icstr) { return GCSsys.calculateConstraintErrorByTag(icstr);}

/// Returns the size of the Geometry
int getGeometrySize(void) {return Geoms.size();}

enum GeoType {
None = 0,
Point = 1, // 1 Point(start), 2 Parameters(x,y)
Expand Down
118 changes: 85 additions & 33 deletions src/Mod/Sketcher/App/SketchObject.cpp
Expand Up @@ -82,6 +82,12 @@ SketchObject::SketchObject()
ExternalGeo.push_back(HLine);
ExternalGeo.push_back(VLine);
rebuildVertexIndex();

lastDoF=0;
lastHasConflict=false;
lastHasRedundancies=false;
lastSolverStatus=0;
lastSolveTime=0;
}

SketchObject::~SketchObject()
Expand Down Expand Up @@ -110,71 +116,105 @@ App::DocumentObjectExecReturn *SketchObject::execute(void)
delConstraintsToExternal();
}

Sketch sketch;

int dofs = sketch.setUpSketch(getCompleteGeometry(), Constraints.getValues(),
// We should have an updated Sketcher geometry or this execute should not have happened
// therefore we update our sketch object geometry with the SketchObject one.
//
// set up a sketch (including dofs counting and diagnosing of conflicts)
lastDoF = solvedSketch.setUpSketch(getCompleteGeometry(), Constraints.getValues(),
getExternalGeometryCount());
if (dofs < 0) { // over-constrained sketch
lastHasConflict = solvedSketch.hasConflicts();
lastHasRedundancies = solvedSketch.hasRedundancies();
lastConflicting=solvedSketch.getConflicting();
lastRedundant=solvedSketch.getRedundant();

if (lastDoF < 0) { // over-constrained sketch
std::string msg="Over-constrained sketch\n";
appendConflictMsg(sketch.getConflicting(), msg);
appendConflictMsg(lastConflicting, msg);
return new App::DocumentObjectExecReturn(msg.c_str(),this);
}
if (sketch.hasConflicts()) { // conflicting constraints
if (lastHasConflict) { // conflicting constraints
std::string msg="Sketch with conflicting constraints\n";
appendConflictMsg(sketch.getConflicting(), msg);
appendConflictMsg(lastConflicting, msg);
return new App::DocumentObjectExecReturn(msg.c_str(),this);
}
if (sketch.hasRedundancies()) { // redundant constraints
if (lastHasRedundancies) { // redundant constraints
std::string msg="Sketch with redundant constraints\n";
appendRedundantMsg(sketch.getRedundant(), msg);
appendRedundantMsg(lastRedundant, msg);
return new App::DocumentObjectExecReturn(msg.c_str(),this);
}

// solve the sketch
if (sketch.solve() != 0)
lastSolverStatus=solvedSketch.solve();
lastSolveTime=solvedSketch.SolveTime;

if (lastSolverStatus != 0)
return new App::DocumentObjectExecReturn("Solving the sketch failed",this);

std::vector<Part::Geometry *> geomlist = sketch.extractGeometry();
std::vector<Part::Geometry *> geomlist = solvedSketch.extractGeometry();
Geometry.setValues(geomlist);
for (std::vector<Part::Geometry *>::iterator it=geomlist.begin(); it != geomlist.end(); ++it)
if (*it) delete *it;

Shape.setValue(sketch.toShape());
// this is not necessary for sketch representation in edit mode, unless we want to trigger an update of
// the objects that depend on this sketch (like pads)
Shape.setValue(solvedSketch.toShape());

return App::DocumentObject::StdReturn;
}

int SketchObject::hasConflicts(void) const
int SketchObject::hasConflicts(void)
{
// TODO: This must be reviewed to see if we are not setting an already set geometry again (and calculating again)
// it is unclear if we need to know if there are conflicts of an updated geometry that has not been already solved
// or not.

// set up a sketch (including dofs counting and diagnosing of conflicts)
Sketch sketch;
int dofs = sketch.setUpSketch(getCompleteGeometry(), Constraints.getValues(),
/*lastDoF = solvedSketch.setUpSketch(getCompleteGeometry(), Constraints.getValues(),
getExternalGeometryCount());
if (dofs < 0) // over-constrained sketch
lastHasConflict = solvedSketch.hasConflicts();
lastHasRedundancies = solvedSketch.hasRedundancies();
lastConflicting=solvedSketch.getConflicting();
lastRedundant=solvedSketch.getRedundant();*/

if (lastDoF < 0) // over-constrained sketch
return -2;
if (sketch.hasConflicts()) // conflicting constraints
if (solvedSketch.hasConflicts()) // conflicting constraints
return -1;

return 0;
}

int SketchObject::solve()
{
// We should have an updated Sketcher geometry or this solver should not have happened
// therefore we update our sketch object geometry with the SketchObject one.
//
// set up a sketch (including dofs counting and diagnosing of conflicts)
Sketch sketch;
int dofs = sketch.setUpSketch(getCompleteGeometry(), Constraints.getValues(),
lastDoF = solvedSketch.setUpSketch(getCompleteGeometry(), Constraints.getValues(),
getExternalGeometryCount());

lastHasConflict = solvedSketch.hasConflicts();

int err=0;
if (dofs < 0) // over-constrained sketch
if (lastDoF < 0) // over-constrained sketch
err = -3;
else if (sketch.hasConflicts()) // conflicting constraints
else if (lastHasConflict) // conflicting constraints
err = -3;
else if (sketch.solve() != 0) // solving
err = -2;
else {
lastSolverStatus=solvedSketch.solve();
if (lastSolverStatus != 0) // solving
err = -2;
}

lastHasRedundancies = solvedSketch.hasRedundancies();

lastConflicting=solvedSketch.getConflicting();
lastRedundant=solvedSketch.getRedundant();
lastSolveTime=solvedSketch.SolveTime;

if (err == 0) {
// set the newly solved geometry
std::vector<Part::Geometry *> geomlist = sketch.extractGeometry();
std::vector<Part::Geometry *> geomlist = solvedSketch.extractGeometry();
Geometry.setValues(geomlist);
for (std::vector<Part::Geometry *>::iterator it = geomlist.begin(); it != geomlist.end(); ++it)
if (*it) delete *it;
Expand Down Expand Up @@ -306,26 +346,38 @@ int SketchObject::toggleDriving(int ConstrId)

int SketchObject::movePoint(int GeoId, PointPos PosId, const Base::Vector3d& toPoint, bool relative)
{
Sketch sketch;
int dofs = sketch.setUpSketch(getCompleteGeometry(), Constraints.getValues(),
// if we are moving a point, we need to start from a solved sketch
// if we have conflicts we can forget about moving

/*lastDoF = solvedSketch.setUpSketch(getCompleteGeometry(), Constraints.getValues(),
getExternalGeometryCount());
if (dofs < 0) // over-constrained sketch
lastHasConflict = solvedSketch.hasConflicts();
lastHasRedundancies = solvedSketch.hasRedundancies();
lastConflicting=solvedSketch.getConflicting();
lastRedundant=solvedSketch.getRedundant();*/

if (lastDoF < 0) // over-constrained sketch
return -1;
if (sketch.hasConflicts()) // conflicting constraints
if (lastHasConflict) // conflicting constraints
return -1;

// move the point and solve
int ret = sketch.movePoint(GeoId, PosId, toPoint, relative);
if (ret == 0) {
std::vector<Part::Geometry *> geomlist = sketch.extractGeometry();
lastSolverStatus = solvedSketch.movePoint(GeoId, PosId, toPoint, relative);

// moving the point can not result in a conflict that we did not have
// or a redundancy that we did not have before, or a change of DoF

if (lastSolverStatus == 0) {
std::vector<Part::Geometry *> geomlist = solvedSketch.extractGeometry();
Geometry.setValues(geomlist);
//Constraints.acceptGeometry(getCompleteGeometry());
for (std::vector<Part::Geometry *>::iterator it=geomlist.begin(); it != geomlist.end(); ++it) {
if (*it) delete *it;
}
}

return ret;
return lastSolverStatus;
}

Base::Vector3d SketchObject::getPoint(int GeoId, PointPos PosId) const
Expand Down
32 changes: 31 additions & 1 deletion src/Mod/Sketcher/App/SketchObject.h
Expand Up @@ -32,6 +32,8 @@
#include <Mod/Part/App/PropertyGeometryList.h>
#include <Mod/Sketcher/App/PropertyConstraintList.h>

#include "Sketch.h"

namespace Sketcher
{

Expand Down Expand Up @@ -104,7 +106,7 @@ class SketcherExport SketchObject : public Part::Part2DObject
std::vector<Part::Geometry*> getCompleteGeometry(void) const;

/// returns non zero if the sketch contains conflicting constraints
int hasConflicts(void) const;
int hasConflicts(void);

/// solves the sketch and updates the Geometry
int solve();
Expand Down Expand Up @@ -190,6 +192,23 @@ class SketcherExport SketchObject : public Part::Part2DObject
bool evaluateConstraints() const;
/// Remove constraints with invalid indexes
void validateConstraints();

/// gets DoF of last solver execution
int getLastDoF() {return lastDoF;}
/// gets HasConflicts status of last solver execution
bool getLastHasConflicts() {return lastHasConflict;}
/// gets HasRedundancies status of last solver execution
bool getLastHasRedundancies() {return lastHasRedundancies;}
/// gets solver status of last solver execution
int getLastSolverStatus() {return lastSolverStatus;}
/// gets solver SolveTime of last solver execution
float getLastSolveTime() {return lastSolveTime;}
/// gets the conflicting constraints of the last solver execution
const std::vector<int> &getLastConflicting(void) const { return lastConflicting; }
/// gets the redundant constraints of last solver execution
const std::vector<int> &getLastRedundant(void) const { return lastRedundant; }

Sketch &getSolvedSketch(void) {return solvedSketch;}

protected:
/// get called by the container when a property has changed
Expand All @@ -201,6 +220,17 @@ class SketcherExport SketchObject : public Part::Part2DObject

std::vector<int> VertexId2GeoId;
std::vector<PointPos> VertexId2PosId;

Sketch solvedSketch;

int lastDoF;
bool lastHasConflict;
bool lastHasRedundancies;
int lastSolverStatus;
float lastSolveTime;

std::vector<int> lastConflicting;
std::vector<int> lastRedundant;

bool AutoLockTangencyAndPerpty(Constraint* cstr, bool bForce = false, bool bLock = true);
};
Expand Down
15 changes: 14 additions & 1 deletion src/Mod/Sketcher/App/SketchObjectPyImp.cpp
Expand Up @@ -241,7 +241,20 @@ PyObject* SketchObjectPy::addConstraint(PyObject *args)
return 0;
}
int ret = this->getSketchObjectPtr()->addConstraint(constr);
this->getSketchObjectPtr()->solve();
// this solve is necessary because:
// 1. The addition of constraint is part of a command addition
// 2. This solve happens before the command is committed
// 3. A constraint, may effect a geometry change (think of coincident,
// a line's point moves to meet the other line's point
// 4. The transaction is comitted before any other solve, for example
// the one of execute() triggered by a recompute (UpdateActive) is generated.
// 5. Upon "undo", the constraint is removed (it was before the command was committed)
// however, the geometry changed after the command was committed, so the point that
// moved do not go back to the position where it was.
//
// N.B.: However, the solve itself may be inhibited in cases where groups of geometry/constraints
// are added together, because in that case undoing will also make the geometry disappear.
this->getSketchObjectPtr()->solve();
return Py::new_reference_to(Py::Int(ret));
}
else if (PyObject_TypeCheck(pcObj, &(PyList_Type)) ||
Expand Down

0 comments on commit 0e92e63

Please sign in to comment.