Geometry_Engine: Add Fillet method for Polylines

[Chrisshort92]

NOTE: Depends on

Issues addressed by this PR

Closes #3609

Rounds every internal corner of a Polyline with a circular arc of the given radius, trimming each adjacent segment back to make room. Corners that can't take the requested radius (too sharp, too close to an end, or running straight through) are capped or left sharp rather than dropped, so the output is always a single contiguous PolyCurve.

Test files

https://burohappold.sharepoint.com/:f:/r/sites/BHoM/02_Current/12_Scripts/02_Pull%20Request/BHoM/BHoM_Engine/Geometry_Engine/%233610-add-fillet-function-for-polylines?d=w4aa8a6b5b8874c5f89f2c8358676f604&csf=1&web=1&e=nMEp3F

Changelog

-Modify.Fillet() added to Geometry_Engine to round corners of polylines, providing the new polycurve

Additional comments

Offset.cs already has a private Fillet(ICurve, ICurve, ...) helper, used internally to reconnect two offset curve segments that have drifted apart. Despite the name, it doesn't round anything: it extends or trims the two existing curves (a Line stays a Line, an Arc keeps its original radius) until they meet at their mutual intersection, producing a sharp, mitred joint. No radius is involved and no new curve is created.

This PR's Fillet(this Polyline, double radius, ...) does something different: given one Polyline and a target radius, it trims each segment back from every corner and inserts a new circular Arc of that radius, tangent to both sides, producing an actually rounded corner and a single continuous curve.

The two share a name by coincidence — the private method carries a stale TODO: make it public comment that this PR does not fulfil as they are target different functions. They solve unrelated problems (sharp-joining two disconnected curves vs. rounding one polyline's corners with a chosen radius), so the private method is left in place rather than replaced.

[Chrisshort92]

@BHoMBot check required

[bhombot-ci]

@Chrisshort92 to confirm, the following actions are now queued:

• check code-compliance
• check documentation-compliance
• check project-compliance
• check core
• check null-handling
• check serialisation
• check versioning
• check installer

[Chrisshort92]

The finding: the expected values in these datasets were generated under .NET Framework and are now being checked on .NET 6, and the harness stores and compares them in a way that can't absorb the difference. That bites twice. Where a test's output is a string containing a number, .NET Core 3.0 changed the default double.ToString() from 15 significant digits to shortest-round-trippable, so the identical double now prints as 6.000262860000012 instead of the stored 6.00026286000001 and the exact string comparison fails. Where the output is numeric, the harness compares with an absolute NumericTolerance = 1e-12 against mm-scale coordinates up to 7.3e6 — magnitudes at which one ULP of a double is already 9.3e-10, so the tolerance is finer than a single bit and the test effectively demands bit-exact reproduction across runtimes. 51 of the 72 numeric failures are 1–3 ULP apart, i.e. ordinary JIT/runtime variation that no code change can fix. Adding Fillet.cs is incidental: the failures reproduce byte-for-byte with it absent.

The one caveat: the runtime switch explains the string diffs and the sub-3-ULP numeric ones, but not the 21 larger differences (PlaneIntersections expecting -1022.77 and getting 1.50). Those are too big to be precision artefacts — likely list ordering — and are a genuine open question for the geometry owners, just not one this PR caused.

[IsakNaslundBh]

The finding: the expected values in these datasets were generated under .NET Framework and are now being checked on .NET 6, and the harness stores and compares them in a way that can't absorb the difference. That bites twice. Where a test's output is a string containing a number, .NET Core 3.0 changed the default double.ToString() from 15 significant digits to shortest-round-trippable, so the identical double now prints as 6.000262860000012 instead of the stored 6.00026286000001 and the exact string comparison fails. Where the output is numeric, the harness compares with an absolute NumericTolerance = 1e-12 against mm-scale coordinates up to 7.3e6 — magnitudes at which one ULP of a double is already 9.3e-10, so the tolerance is finer than a single bit and the test effectively demands bit-exact reproduction across runtimes. 51 of the 72 numeric failures are 1–3 ULP apart, i.e. ordinary JIT/runtime variation that no code change can fix. Adding Fillet.cs is incidental: the failures reproduce byte-for-byte with it absent.

The one caveat: the runtime switch explains the string diffs and the sub-3-ULP numeric ones, but not the 21 larger differences (PlaneIntersections expecting -1022.77 and getting 1.50). Those are too big to be precision artefacts — likely list ordering — and are a genuine open question for the geometry owners, just not one this PR caused.

quick comment on this:

It is a known issue, that ofc needs to be resolved properly. In the meantime, though, if you re-create the unittests in RHino 7 (or set GH for rhino 8 to run in framework) and reserialise them, then they should be passing on the bot. To get this PR merged that will be unfortunately be required.

[peterjamesnugent]

Covered in the documentation here:
https://bhom.xyz/documentation/Guides-and-Tutorials/Coding-with-BHoM/Testing/Data-Driven-Tests/#storing-test-data-for-engine-methods

[Chrisshort92]

@BHoMBot check required

[bhombot-ci]

@Chrisshort92 to confirm, the following actions are now queued:

• check code-compliance
• check documentation-compliance
• check project-compliance
• check core
• check null-handling
• check serialisation
• check versioning
• check installer

There are 30 requests in the queue ahead of you.

[Martian42]

Successfully replicate the intended output using the test script. Happy to merge.

[Chrisshort92]

@BHoMBot copyright-compliance

[bhombot-ci]

@Chrisshort92 sorry, I didn't understand.
Was that comment an instruction for me? If so, could you state again what check you would like me to do?
For a list of available instructions, please see this wiki page.

[Chrisshort92]

@BHoMBot check copyright-compliance

[bhombot-ci]

@Chrisshort92 to confirm, the following actions are now queued:

• check copyright-compliance

There are 9 requests in the queue ahead of you.

[Chrisshort92]

@BHoMBot check dataset-compliance

[bhombot-ci]

@Chrisshort92 to confirm, the following actions are now queued:

• check dataset-compliance

There are 9 requests in the queue ahead of you.

[Chrisshort92]

@BHoMBot check unit-tests

[bhombot-ci]

@Chrisshort92 to confirm, the following actions are now queued:

• check unit-tests

There are 11 requests in the queue ahead of you.

[peterjamesnugent]

Comments below.

In your test script this fillet is only 1.719 whilst the input is 1.83:

Can you clean up the comments in the file please - some seem like reasoning and should be cleaned up for merging.

Also, does your test script/unit test cover most cases? Is there one for covering where the radius is greater than the line segment? What about short segments where the points are near co-incident?

[Chrisshort92]

Comments below.

In your test script this fillet is only 1.719 whilst the input is 1.83:

Can you clean up the comments in the file please - some seem like reasoning and should be cleaned up for merging.

Also, does your test script/unit test cover most cases? Is there one for covering where the radius is greater than the line segment? What about short segments where the points are near co-incident?

Method used to terminate at half a segments length, which the case you highlight is doing. I have now edited the method to terminate once two arc meet, you can see this play out below.

[Chrisshort92]

@BHoMBot check ready-to-merge

[bhombot-ci]

@Chrisshort92 to confirm, the following actions are now queued:

• check ready-to-merge

There are 99 requests in the queue ahead of you.

[bhombot-ci]

The check ready-to-merge has already been run previously and recorded as a successful check. This check has not been run again at this time.

[IsakNaslundBh]

Reviewed this in a call with @peterjamesnugent and think it seem to be working well.

Only thing I would change is the below.

Will just put as a comment review so that this can be re-approved without having to rely on me once this has been fixed.

[IsakNaslundBh]

Missed the comment

[Chrisshort92]

@BHoMBot check required

[bhombot-ci]

@Chrisshort92 to confirm, the following actions are now queued:

• check code-compliance
• check documentation-compliance
• check project-compliance
• check core
• check null-handling
• check serialisation
• check versioning
• check installer

There are 10 requests in the queue ahead of you.

[peterjamesnugent]

Tested this, and the change has been implemented.

Unit tests needs to be added for 0 radii.

[Chrisshort92]

@BHoMBot check required

[bhombot-ci]

@Chrisshort92 to confirm, the following actions are now queued:

• check code-compliance
• check documentation-compliance
• check project-compliance
• check core
• check null-handling
• check serialisation
• check versioning
• check installer

There are 12 requests in the queue ahead of you.

[bhombot-ci]

The check versioning has already been run previously and recorded as a successful check. This check has not been run again at this time.

[bhombot-ci]

The check installer has already been run previously and recorded as a successful check. This check has not been run again at this time.

[Chrisshort92]

@BHoMBot unit-tests

[bhombot-ci]

@Chrisshort92 sorry, I didn't understand.
Was that comment an instruction for me? If so, could you state again what check you would like me to do?
For a list of available instructions, please see this wiki page.

[Chrisshort92]

@BHoMBot check copyright-compliance

[bhombot-ci]

@Chrisshort92 to confirm, the following actions are now queued:

• check copyright-compliance

There are 9 requests in the queue ahead of you.

[Chrisshort92]

@BHoMBot check dataset-compliance

[bhombot-ci]

@Chrisshort92 to confirm, the following actions are now queued:

• check dataset-compliance

There are 9 requests in the queue ahead of you.

[Chrisshort92]

@BHoMBot check unit-tests

[bhombot-ci]

@Chrisshort92 to confirm, the following actions are now queued:

• check unit-tests

There are 11 requests in the queue ahead of you.

[Chrisshort92]

@BHoMBot check ready-to-merge

[bhombot-ci]

@Chrisshort92 to confirm, the following actions are now queued:

• check ready-to-merge

There are 13 requests in the queue ahead of you.

[peterjamesnugent]

Changes addressed - happy to merge.

[peterjamesnugent]

@BHoMBot check ready-to-merge

[bhombot-ci]

@peterjamesnugent to confirm, the following actions are now queued:

• check ready-to-merge

There are 9 requests in the queue ahead of you.