BHoM_Engine icon indicating copy to clipboard operation
BHoM_Engine copied to clipboard

Geometry_Engine: useInfiniteLines argument is ambiguous.

Open pawelbaran opened this issue 7 years ago • 1 comments
trafficstars

The useInfiniteLines argument used in intersection methods is currently applied in two different ways:

In case of Line.PlaneIntersection (the first intersection method that had been implemented), it is used as: useInfiniteLine &= line.Infinite; which means that the line is considered infinite only when both the Infinite property of the object is switched to true and the useInfiniteLines argument is true.

In case of Curve.LineIntersection (the methods written later on): Line l = line.Clone(); l.Infinite |= useInfiniteLine; which can be expressed as the line being infinite either when its Infinite property is true or useInfiniteLine argument is true (all lines automatically become infinite in this case).

First of all, these are two different approaches. Secondly, it seems that in both cases the useInfiniteLine argument could be removed as it somehow doubles the information stored in Line.Infinite.

Personally I see no need for keeping useInfiniteLine in the first case, where it is actually an overhead (one needs to first switch Line.Infinite and then pass useInfiniteLine, why?). On the other hand, I believe that in the second case it has a lot of sense as one does not need to bother about switching Line.Infinite (but still can do it if needed, e.g. in case of intersecting a finite line with an infinite one).

Therefore, I would suggest applying a general principle of l.Infinite |= useInfiniteLine;.

What do you think?

pawelbaran avatar Apr 18 '18 08:04 pawelbaran

As part of the 2024 kick-off today, I have examined this historic issue and found the following information.

LineIntersection and PlaneIntersection for lines are performing as @pawelbaran described back in 2018, indicating that no changes have happened to the code in this regard in the intervening years.

A historic commend by @adecler says

Agreed, l.Infinite |= useInfiniteLine; is the only one that makes sense to me too.

Having looked at the code myself, I would also agree with @adecler that using the |= makes most sense to allow the line object property to have a say in the calculation as well as allowing the user to override that with useInfiniteLines if its appropriate to do so.

Therefore, to close out this issue, the following action should be undertaken:

  • [ ] Refactor methods above to use line.Infinite |= useInfiniteLines rather than &= or other approaches.

FraserGreenroyd avatar Jan 04 '24 09:01 FraserGreenroyd