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

fix inserting break lines in dual edge triangulation #39218

Merged
merged 4 commits into from
Oct 8, 2020

Conversation

vcloarec
Copy link
Member

@vcloarec vcloarec commented Oct 7, 2020

fix #30549 and more generally inserting break line in dual edge triangulation.
Hope it is the last issue... (surely not!)

@vcloarec vcloarec added Bug Either a bug report, or a bug fix. Let's hope for the latter! Crash/Data Corruption Processing Relating to QGIS Processing framework or individual Processing algorithms labels Oct 7, 2020
@vcloarec vcloarec mentioned this pull request Oct 7, 2020
@github-actions github-actions bot added this to the 3.16.0 milestone Oct 7, 2020
@@ -1327,13 +1327,39 @@ unsigned int QgsDualEdgeTriangulation::insertEdge( int dual, int next, int point

}

static bool triangleIsFlat( const QgsPoint &pt1, const QgsPoint &pt2, const QgsPoint &pt3, double tolerance )
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this method could be useful in QgsTriangle class?

Copy link
Member

Choose a reason for hiding this comment

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

Or in QgsGeometryUtils to avoid the creation of a triangle each time

Copy link
Member Author

Choose a reason for hiding this comment

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

Ad this function is not complete (but sufficient in this context), no sure. Indeed, only the distance between the segment [pt1,pt2] from pt3 is checked. So if this segment is very small and pt3 far away, the triangle is flat but the function does not return true.
This is acceptable here and changing that will slow down the function. I think I will change the function name / add comments.

Copy link
Collaborator

@nyalldawson nyalldawson left a comment

Choose a reason for hiding this comment

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

Pretty tricky to review this one, but I can't see any glaring issues!

@nyalldawson nyalldawson merged commit 2141593 into qgis:master Oct 8, 2020
@vcloarec vcloarec deleted the fixBreakLine_2 branch October 8, 2020 23:23
@vcloarec
Copy link
Member Author

vcloarec commented Oct 8, 2020

@nyalldawson , also pretty tricky to fix...

@nyalldawson
Copy link
Collaborator

I can imagine! I'm just glad that these classes are getting some love and no longer produce embarrassing results!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! Crash/Data Corruption Processing Relating to QGIS Processing framework or individual Processing algorithms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TIN creation crash
3 participants