-
-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Feature/add chamfer fillet #62572
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
base: master
Are you sure you want to change the base?
Feature/add chamfer fillet #62572
Conversation
…tilsBase Add createChamfer() and createFillet() methods for 2D geometric operations. These low-level functions handle segment intersection, angle calculation, and tangent point computation for CAD-style corner modifications.
Add vertex-based and segment-based chamfer() and fillet() methods with Z/M coordinate interpolation support. Handles both LineString modification and standalone segment operations for CAD-style corner editing.
…operations Handle vertex-based operations on CompoundCurve geometries by preserving existing CircularString segments and properly inserting new chamfer/fillet geometry without segmentation.
🪟 Windows buildsDownload Windows builds of this PR for testing. 🍎 MacOS Qt6 buildsDownload MacOS Qt6 builds of this PR for testing. 🪟 Windows Qt6 buildsDownload Windows Qt6 builds of this PR for testing. |
481c502
to
78972f6
Compare
* \param epsilon tolerance for numerical comparisons and intersection detection | ||
* \returns TRUE if fillet was successfully created | ||
* | ||
* \note The caller must ensure that filletPointsX and filletPointsY arrays are |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how is that working with the Python bindings? Might be safer to sip_skip this one...
double *trim1StartX = nullptr, double *trim1StartY = nullptr, | ||
double *trim1EndX = nullptr, double *trim1EndY = nullptr, | ||
double *trim2StartX = nullptr, double *trim2StartY = nullptr, | ||
double *trim2EndX = nullptr, double *trim2EndY = nullptr, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't these all be SIP_OUT too?
* \param y y-coordinate of the point to interpolate | ||
* \param segStart start point of the segment | ||
* \param segEnd end point of the segment | ||
* \param distanceFromStart distance from segment start to the interpolated point |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't this be automatically calculated based on x/y ?
* Interpolates a point on a segment with proper Z and M value interpolation. | ||
* \param x x-coordinate of the point to interpolate | ||
* \param y y-coordinate of the point to interpolate | ||
* \param segStart start point of the segment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change "seg" to "segment" everywhere -- as per https://github.com/qgis/QGIS-Enhancement-Proposals/blob/master/qep-314-coding-style.md#1-naming 1.3
|
||
/** | ||
* Creates a fillet (rounded corner) between two line segments using QgsPoint. | ||
* Returns the three fillet arc points via output parameters. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume this is a circular arc? I think it should be explicitly mentioned in the docs
distance2 = distance1; | ||
|
||
// Validate input parameters | ||
if ( distance1 < 0 || distance2 < 0 ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if ( distance1 < 0 || distance2 < 0 ) | |
if ( distance1 <= 0 || distance2 <= 0 ) |
if ( !createChamfer( seg1Start, seg1End, seg2Start, seg2End, distance1, distance2, chamferStart, chamferEnd ) ) | ||
return nullptr; | ||
|
||
QgsLineString *completeLine = new QgsLineString(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use unique_ptr. Also use constructor for QgsLineString which takes a list of points instead of calling addVertex multiple times
if ( segments <= 0 ) | ||
{ | ||
// Return CompoundCurve with circular arc | ||
QgsCompoundCurve *completeCurve = new QgsCompoundCurve(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unique_ptr (elsewhere too)
QgsCompoundCurve *completeCurve = new QgsCompoundCurve(); | ||
|
||
// First linear segment | ||
QgsLineString *firstSegment = new QgsLineString(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as above, use constructor instead of multiple addVertex calls (elsewhere too)
|
||
QgsGeometry QgsGeometry::chamfer( int vertexIndex, double distance1, double distance2 ) const | ||
{ | ||
const QgsCurve *curve = qgsgeometry_cast<const QgsCurve *>( d->geometry.get() ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want to be more tolerant and also accept multicurves with a single geometry:
const QgsCurve *curve = qgsgeometry_cast<const QgsCurve *>( d->geometry.get() ); | |
if ( isNull() ) | |
return QgsGeometry(); | |
const QgsCurve *curve = qgsgeometry_cast<const QgsCurve *>( d->geometry->simplifiedTypeRef() ); |
elsewhere too
@lbartoletti nice work! Above review is about the api and high-level considerations only... I haven't reviewed the actual maths behind the chamfer/fillet geometry creation. I trust you on those! 😆 |
Can you elaborate on the plans for exposing these? My personal preference is that they'd be implemented via QgsAdvancedDigitizingToolsRegistry and exposed only in the CAD dock tools panel, as that's the approach with the least ui complexity cost. |
QgsGeometry fillet( const QgsPoint &seg1Start, const QgsPoint &seg1End, | ||
const QgsPoint &seg2Start, const QgsPoint &seg2End, | ||
double radius, int segments = 8 ) const; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain why those variants using the segment points are needed?
To me they look weird, as it super easy to call with points that do not match segments or even do not lie on the geometry!
Thank you! |
Description
This PR introduces core functionality for chamfer and fillet operations on geometric objects, commonly used in CAD applications for corner modification.
There are two implementation level:
Low-level 2D operations (
QgsGeometryUtilsBase
):createChamfer()
- Creates chamfer between two segments (x/y coordinates) with specified distancescreateFillet()
- Creates circular fillet between two segments (x/y coordinates) with specified radiusHigh-level geometry operations (
QgsGeometry
):chamfer(vertexIndex, ...)
andfillet(vertexIndex, ...)
chamfer(seg1, seg2, ...)
andfillet(seg1, seg2, ...)
The algorithms support both touching and non-touching segments.
Some visual examples:
Original:

Symetric chamfer:

Asymetric Chamfer:

Fillet (preserving arc here):

Future Work
The map tools will be added in a separate PR
Maybe a processing too
Funded by: Frankurt.