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

New classes for circle and ellipse. #4358

Closed
wants to merge 316 commits into from
Closed

New classes for circle and ellipse. #4358

wants to merge 316 commits into from

Conversation

lbartoletti
Copy link
Member

Two new classes from CADDigitze plugin.

Circle and ellipse have z (and m) value(s) but without inclination (for now).

double yDelta_b = pt3.y() - pt2.y();
double xDelta_b = pt3.x() - pt2.x();

if ( ( fabs( xDelta_a ) <= epsilon ) && ( fabs( yDelta_b ) <= epsilon ) )
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use qAbs for better cross platform compatibility


if ( !isPerpendicular( pt1, pt2, pt3, epsilon ) )
{
p1 = pt1 ; p2 = pt2; p3 = pt3;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should be separate lines

double aSlope = yDelta_a / xDelta_a;
double bSlope = yDelta_b / xDelta_b;

if ( ( fabs( xDelta_a ) <= epsilon ) && ( fabs( yDelta_b ) <= epsilon ) )
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

qAbs


if ( ( fabs( xDelta_a ) <= epsilon ) && ( fabs( yDelta_b ) <= epsilon ) )
{
std::cout << "par ici" << std::endl;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove debugging line

QgsCircle();

/** Constructs a circle by defining all the members.
* @param center The center of the circle.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you flip these to \param notation? We're trying to standardise on that format now

* \class QgsEllipse
* \brief Ellipse geometry type.
*
* An ellipse is defined by a center point (mCenter) with a semi-major axis, a semi-minor axis and an azimuth.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to keep the internal variable names out of the API docs (since they aren't accessible through the python bindings)

* @see setCenter()
* @note not available in Python bindings
*/
QgsPointV2 &rcenter() { return mCenter; }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're using sipify you'll need to wrap this in

#ifndef SIP_RUN

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SIP_SKIP should do the job
like here

* @param pts List of points returned.
* @param segments Number of segments used to segment geometry.
*/
virtual void points( QgsPointSequence &pts, unsigned int segments = 36 ) const;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why not just the sequence instead of using a reference?

/** Returns a segmented polygon.
* @param segments Number of segments used to segment geometry.
*/
virtual QgsPolygonV2 toPolygon( unsigned int segments = 36 ) const;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think given that the main use for toPolygon and toLineString will be store the result in a QgsGeometry that these should be factories and return a pointer. It'll avoid the need for an extra clone to store the result in QgsGeometry.


/** Returns the oriented minimal bounding box for the ellipse.
*/
virtual QgsPolygonV2 orientedBoundingBox() const;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here - this should return a pointer

@nyalldawson
Copy link
Collaborator

Fantastic work as usual!

nyalldawson and others added 20 commits April 17, 2017 09:28
Skip the autocreated Docstrings with rtype annotations if the
member is overridden - better to use the base class
Docstring in this case.
Allows argument names to be highlighted in Python docs
These classes have been replaced by more efficient and flexible classes
(see QgsPointLocator, QgsSnappingUtils, QgsMapCanvasSnappingUtils)

* get rid of params to get rid of warnings
[needs-docs] use QgsPasswordLineEdit in the master password dialog
- remove methods which accept a QgsGeometry pointer, leave
duplicate methods which accept QgsGeometry reference (caller
should handle null geometry pointers in an acceptable
way instead of just returning 0 measurements)
- make protected members private. This class isn't designed
to be subclassed
- remove some methods not exposed to bindings and not used
anywhere
nyalldawson and others added 9 commits April 26, 2017 13:56
This temporary layer store (a QgsProject) is used as a
store for layers that are added if a parameter that
is evaluated to a layer requires that a new, non-active-project
layer is loaded. It means that these layers will remain accessible
for the duration of the algorithm's execution (or models
execution if an algorithm is run as part of a model), before
being automatically discarded when the QgsProcessingContext
used to run the algorithm/model goes out of scope.

This approach has several benefits:
- it means that algorithms (including c++ algorithms) are able
to use both project and non-project layers without needing
to handle any memory management themselves.
- it means that layers are guaranteed to last for the duration
of a model execution. This is currently an issue where models
which use memory layers as intermediate outputs do not
function correctly as the memory layers are destroyed before
the model has finished executing
- there should be no leakage of layers remaining open
after an algorithm exits
[processing] Move some log handling to c++ class
 * add Array and ArraySize annotations
 * also handle multiline skipped bodies
 * handle #if 0 blocks
@nyalldawson
Copy link
Collaborator

@lbartoletti looks like you need to rebase this and re-run the updated sipify_all to update the python bindings.

In your opinion is this ready to go now?

nyalldawson and others added 7 commits April 26, 2017 17:37
Allow taking layers from QgsProject
 * add In annotation
 * remove struct forward declarations
 * fix members initialization list in header
 * merge removal code for function bodies and constructor definition
 * skip some operators
 * allow to remove an argument with SIP_PYARGREMOVE
@lbartoletti
Copy link
Member Author

@nyalldawson Yes, if it's ok for you, it's ok for me.

I will improve this classes with the inclination after the class for regular polygons and possibly other CAD (geometry utils) tools

@nyalldawson
Copy link
Collaborator

Manually squashed and merged as 84471f2

@nyalldawson
Copy link
Collaborator

Many thanks @lbartoletti !

@lbartoletti lbartoletti deleted the circle_ellipse branch April 27, 2017 04:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet