Geometry_Engine: IsRectangle and LongestEdgeDirection added#3588
Open
WojciechBuda00 wants to merge 6 commits into
Open
Geometry_Engine: IsRectangle and LongestEdgeDirection added#3588WojciechBuda00 wants to merge 6 commits into
WojciechBuda00 wants to merge 6 commits into
Conversation
WojciechBuda00
requested review from
IsakNaslundBh,
al-fisher and
pawelbaran
as code owners
pawelbaran
reviewed
May 22, 2026
| [Input("angleTolerance", "Angular tolerance used when comparing edge directions for parallelism.")] | ||
| [Output("direction", "Direction vector of the longest edge.")] | ||
|
|
||
| public static Vector LongestEdgeDirection(this Polyline polyline, double tolerance, double angleTolerance = Tolerance.Angle) |
Member
There was a problem hiding this comment.
Suggested change
| public static Vector LongestEdgeDirection(this Polyline polyline, double tolerance, double angleTolerance = Tolerance.Angle) | |
| public static Vector LongestEdgeDirection(this Polyline polyline, double tolerance = Tolerance.Distance, double angleTolerance = Tolerance.Angle) |
pawelbaran
requested changes
May 26, 2026
| foreach (Line edge in edges) | ||
| { | ||
| Vector direction = edge.Direction(); | ||
| direction.Z = 0; |
Member
There was a problem hiding this comment.
Do not project the direction on XY, the method should work for any polyline
Comment on lines
+55
to
+56
| if (direction.Length() <= tolerance) | ||
| continue; |
Member
There was a problem hiding this comment.
I do not think this is needed, should be implicitly eliminated by edge length filter ln46
Comment on lines
+58
to
+61
| if (direction.X < 0 || (Math.Abs(direction.X) < tolerance && direction.Y < 0)) | ||
| { | ||
| direction = -direction; | ||
| } |
Member
There was a problem hiding this comment.
No need to flip the direction vector, IsParallel will work regardless
|
|
||
| if (existDir != null) | ||
| { | ||
| dirLen[existDir] = edge.Length(); |
Member
There was a problem hiding this comment.
My idea was to find the dominant edge direction, not the direction of a single longest edge. May have miscommunicated that originally
Suggested change
| dirLen[existDir] = edge.Length(); | |
| dirLen[existDir] += edge.Length(); |
| [Input("angleTolerance", "Angular tolerance used when comparing edge directions for parallelism.")] | ||
| [Output("direction", "Direction vector of the longest edge.")] | ||
|
|
||
| public static Vector LongestEdgeDirection(this Polyline polyline, double tolerance = Tolerance.Distance, double angleTolerance = Tolerance.Angle) |
Member
There was a problem hiding this comment.
I think we may have missed each other when talking about this method's functionality: what I meant was not the direction of a single longest edge, but rather dominant direction in a polyline, i.e. a sum of lengths of all parallel edges. I think the method name should reflect this
Suggested change
| public static Vector LongestEdgeDirection(this Polyline polyline, double tolerance = Tolerance.Distance, double angleTolerance = Tolerance.Angle) | |
| public static Vector DominantEdgeDirection(this Polyline polyline, double tolerance = Tolerance.Distance, double angleTolerance = Tolerance.Angle) |
...also please remember to tweak the descriptions accordingly
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
NOTE: Depends on
Issues addressed by this PR
Closes #3586
Test files
Changelog
Additional comments