Refactor LineIntersector
This PR refactors the LineIntersector class to reduce its responsibilities and make it easier to extend it to variable coordinate dimensions using method overloads. Currently, the LineIntersector class is responsible for:
- Storing configuration (the precision model under which operations should take place)
- Storing inputs to (some) intersection operations
- Storing results to (some) intersection operations
The storage of inputs and outputs makes LineIntersector a convenient way to move line segments through algorithms. It can also be make its usage obscure; an intersection may be performed at a location distant in the call stack from when the input segments, intersection points, or intersection type are later retrieved.
This PR proposes to simplify LineIntersector by:
- storing only configuration (i.e., the
PrecisionModelto be used for rounding coordinates) - having all methods return an
IntersectionResultobject that can be queried for the nature of the intersection and any intersection points
The caller will be responsible for storing input segments if needed. This functionality was not used extensively in GEOS and the required changes in client code to drop it were minimal.
I think these changes
- make client code somewhat easier to understand (we can follow an
IntersectionResultobject back through the code to find the operation that created it) - make the
LineIntersectoritself easier to understand (we are no longer manipulating global variables likeisProperVarin nested function calls) - will make it easier to extend the class to difference coordinate types (XY, XYZM, etc.) without requiring that the intersector be templated on coordinate type (as I think would be the case if we stored coordinates of different types).
I had some concern about an adverse impact on performance (we are always returning an object with intersection points, even in the case of no intersection). However, I'm not able to detect any performance impact on my platform.
As an initial "what-if" comment, since the LineIntersector is now just a container for a PrecisionModel, what if the PrecisionModel is the value passed around, and LineIntersector and IntersectionResult are merged into a single class (say LineIntersection)? So when an intersection or intersects computation is needed, a LineIntersection object is created (perhaps with a provided PrecisionModel), and it computes the result on creation.
This will require more refactoring, I suspect - not sure how much. It would make the dependency (or not) on a PrecisionModel clearer.
Another question is whether there is a performance gain to had in situations where only the information about the nature of an intersection is needed, and not the intersection points themselves.
As an initial "what-if" comment, since the LineIntersector is now just a container for a PrecisionModel, what if the PrecisionModel is the value passed around, and LineIntersector and IntersectionResult are merged into a single class (say LineIntersection)? So when an intersection or intersects computation is needed, a LineIntersection object is created (perhaps with a provided PrecisionModel), and it computes the result on creation.
I think it's reasonable for LineIntersector to encapsulate whatever configuration exists for segment intersection. Currently that is just the PrecisionModel but I can see having some choice over whether and how interpolation is performed. But I am also biased towards less invasive changes.
The original motivation here was to better position LineIntersector to handle multiple coordinate dimensions, but since that functionality has since been added and this PR no longer applies to main, it doesn't seem worthwhile at the moment.