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

Geometry_Engine: Improve Offset method (includes Fillet() and ExtendToPoint()) #1333

Open
LMarkowski opened this issue Nov 18, 2019 · 5 comments
Labels
severity:low Doesn't stop/slow current workflow type:feature New capability or enhancement

Comments

@LMarkowski
Copy link
Contributor

Description:

Following the discussion in PR #1314 following items should be improved in Offset() method for Polyline and PolyCurve classes:

  • Offset value below which method is split into two offsets one by one. For now it is arbitrarily set as 0.05 * Curve.Length(). Instead should be calculated or exposed as an input parameter. Link to the specific comment in a PR.
  • The way errors from nested methods are being erased. Currently it uses Reflection.Compute.ClearCurrentEvents(). Probably there is a better way to cover it. Link to the specific comment in a PR. Refers also to Reflection_Engine: possibility to clear events that occurred in nested methods #1306.
  • Fillet() now works only on pairs of Lines and Arcs and rather trims/extends them to the intersection point than really fillet. It was inspired by Autocad Fillet method. Could be improved and added as separate public method or maybe renamed.
  • ExtendToPoint() as above. Extend(curve, double, double) is implemented for most of ICurve classes, only Nurbs are missing. Now we could decide if separate method for extending to a given point instead of extending by a given length would be useful. If so, there is a private method working on lines and arcs in Offset.cs file. It could be used as a draft.
@LMarkowski LMarkowski added severity:low Doesn't stop/slow current workflow type:feature New capability or enhancement labels Nov 18, 2019
@LMarkowski
Copy link
Contributor Author

LMarkowski commented Nov 18, 2019

Besides what is said in the description there are many other ways in which Offest could be improved to be more error-proof. Here are some of my not implemented ideas:

  • first offset every SubPart. Extend them to maximum (Lines -> Infinite and maybe Arcs -> Circles) and split in intersection points. Then the logic from BooleanUnion could be adapt to find biggest possible polygon. Cons: works only on closed curves and positive values of offset. Pros: could work better than what we have now.
  • after offsetting SubParts search for intersections. If they occur, cull every item between two intersecting curves.
  • one of the main obstacles was to decide which ends of filleted curve should be kept. By default it would be curve[i].StartPoint and curve[i+1].EndPoint. It was not always possible but even so it doesn't always mean that this exact curve should be culled.
  • generally imo method for Polylines is much more reliable. Referring to it while offsetting PolyCurve could be considered as often as possible.

@LMarkowski
Copy link
Contributor Author

@pawelbaran @IsakNaslundBh @FraserGreenroyd here's the issue mentioned in Friday's PR.

@FraserGreenroyd
Copy link
Contributor

Great, cheers @LMarkowski 😄

Now just need to find someone to resolve it... 🤔

@FraserGreenroyd
Copy link
Contributor

#1868 is relevant to this issue when this issue gets addressed.

@albinber
Copy link
Contributor

albinber commented Jan 4, 2024

#2503 is also relevant for this issue whenever it gets addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
severity:low Doesn't stop/slow current workflow type:feature New capability or enhancement
Projects
None yet
Development

No branches or pull requests

3 participants