Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sketcher diagnose autoconstraint improvements #1554

Conversation

@abdullahtahiriyo
Copy link
Contributor

commented Jul 14, 2018

Not for direct merge, but for review.

The code works, but I wonder if there would be a better way to expose the analysis functionality to python, not necessarily using the sketch object as middleman. The advantage could be that analysis objects could be:
a) created separated from the analysis object that the sketchobject has as a pointer inside, allowing to have different analysis objects keeping track of different aspects, even running in separate threads with appropriate code to avoid race conditions.

b) interfaced from Python like "ActiveSketch.Analyser.detectHorizontalConstraints()" instead of "ActiveSketch.detectHorizontalConstraints()"

c) not bloating the SketchObject c++/python class with commands that are just to act like a middleman.

A series of algorithms to detect missing constraints and create them.

Each of the algorithms is divided in different steps for maximum flexibility:

    /// There is a first type of routines, simple routines, which work in the following order:
    /// Detect - (Analyse) - [Get] - [Set] - Make
    ///
    /// The Detect step just identifies possible missing constraints.
    ///
    /// The Analyse, which is not available for all the routines, operates in detected constraints of the same routine, to
    /// look for alternatives. For example, a general pointonpoint detection leads to a search for coincident constraints, which
    /// can be later run via Analyse if it is intended to convert endpoint coincidence to endpoint perpendicular and tangent constraints.
    ///
    /// The Get retrieves the result of the analysis as a vector of ConstraintIds, indicating the suggested constraints. This step is intended
    /// for enabling the user to check the result of the analysis, rather than applying it. If only applying is intended, this step is not necessary
    /// as the Make will operate on the result of the Detect - Analyse directly.
    ///
    /// The Set changes the detected result. It modifies the SketchAnalysis object. It only modifies the SketchObject as far as the SketchAnalysis is changed.
    /// It does not apply any changes to the sketch. It is intended so as to enable the user to change the result that will be applied.
    ///
    /// Neither the Detect, nor the Analyse, nor the Get steps modify the Sketch geometry.
    ///
    /// Make applies the constraints stored internally in the SketchAnalysis object.

It includes an automatic constraining algorithm for coincidences, horizontals/verticals and equality:

    /// A second type of routines, complex routines, are thought for running fully automatic and they Detect, Analyse and Make.
    /// They may also apply a variaty of types of Constraints.

It also includes some helper functions, like autoRemoveRedundants
Apart for the inclusion of the Analysis functionality, SketchObject has been improved to provide:

- A fast painless deleteAllConstraints() function

- A fast painless constraint group deletion, delConstraints(std::vector<int> ConstrIds, bool updategeometry)
Exposure of all the functionality in the previous commits to Python
@abdullahtahiriyo

This comment has been minimized.

@wwmayer

This comment has been minimized.

Copy link
Contributor

commented Jul 29, 2018

It's indeed a bit odd to have many new methods added to SketchObject just to have a convenient Python interface. Since the "analyser" member doesn't do anything else and to keep things a bit cleaner you can:

  • keep the functions of SketchObjectPy but create each time a new instance of SketchAnalysis when one of these Python functions is invoked. This way you can remove the analyser member from SketchObject and all the methods. Also, this reduces possible side-effects in case of consecutive calls of validation functions unless you explicitly need this. But then it suffices to add a single method to SketchObject to return the SketchAnalysis instance.

  • instead of putting the validation functions to SketchObjectPy you can create the class SketchAnalysisPy. In the long term this might be the better option since many new methods could be added. In order to invoke the validation functions from Python we have to add a single method to SketchObject(Py) to create and return a SketchAnalysis instance.

Nevertheless, I am going to merge the PR now and things can still be changed at a later time.

@wwmayer

This comment has been minimized.

Copy link
Contributor

commented Jul 29, 2018

Merged.

@wwmayer wwmayer closed this Jul 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.