Add Angle.sinSnap and .cosSnap to avoid small errors, e.g. with buffer operations

[mwtoews]

This is similar to https://github.com/libgeos/geos/pull/978 which aims to remove small rounding errors from Math.sin and Math.cos with (e.g.) pi and pi/2, which are near-zero.

This will provide small differences to buffer operations. For example, consider a 1.0 buffer around POINT (0 0). An intersection with LINESTRING (1 1, 1 -1, -1 -1, -1 1, 1 1) will now be MULTIPOINT ((-1 0), (0 -1), (0 1), (1 0)).

Previously the intersection was MULTIPOINT ((-1 -0.0000000000000001), (-0.0000000000000002 1), (0.0000000000000001 -1), (1 0)).

Several notes:

• I haven't coded in Java for over 20 years, so better coding hints are welcome
• Unlike GEOS, Angle.sinCos returns an array. Perhaps there is a better way?
• Unlike GEOS, there is no BufferOpTest to check the coordinate sequence of POINT(0 0) with a buffer of 1. Is there a suitable place to check this?

[dr-jts]

• Unlike GEOS, Angle.sinCos returns an array. Perhaps there is a better way?

I think two separate functions would be best. Probably more optimal than allocating a new array each time.

• Unlike GEOS, there is no BufferOpTest to check the coordinate sequence of POINT(0 0) with a buffer of 1. Is there a suitable place to check this?

There is BufferTest. Note that the code there is pretty old. For a more modern pattern for testing the results of operations see this code .

[dr-jts]

Is it worth snapping sin and cos values near 0.5 as well?

[mwtoews]

Do you mean something like public static double clipSin(double ang) and Angle.clipCos? I know that C-based would optimise the pair of functions with FSINCOS, but I'm not sure if Java would/wouldn't do that. It might also not matter (there are many other expensive components to a buffer op than these trig functions).

As for trig values near 0.5, I've not found this to be important. It's only the near-zero ones that matter, which is why I went with the simplest implementation for GEOS. I should also note that this PR would make a buffer around point equal to GEOS (again).

[dr-jts]

Do you mean something like public static double clipSin(double ang) and Angle.clipCos? I know that C-based would optimise the pair of functions with FSINCOS, but I'm not sure if Java would/wouldn't do that. It might also not matter (there are many other expensive components to a buffer op than these trig functions).

Yes, that's what I mean. And agreed, I don' think the performance is going to be a bottleneck.

As for trig values near 0.5, I've not found this to be important. It's only the near-zero ones that matter, which is why I went with the simplest implementation for GEOS.

Sounds good.

I should also note that this PR would make a buffer around point equal to GEOS (again).

Excellent, that's what we like to have.

[mwtoews]

This PR is rewritten as discussed, and ready for further review.

[dr-jts]

What do you think about renaming these sinSnap and cosSnap? I think the functionality is more akin to snapping than clipping.

[mwtoews]

Good suggestion. Also, if there are suggestions to push back onto GEOS dev, I can entertain these ideas too.

There are several other parts of the codebase that use Math.sin and .cos that could potentially benefit with clipping near zero, but I haven't gone too far beyond the changes to align it to GEOS.

[dr-jts]

Good suggestion. Also, if there are suggestions to push back onto GEOS dev, I can entertain these ideas too.

Might be nice to align GEOS naming with the changed JTS naming.

There are several other parts of the codebase that use Math.sin and .cos that could potentially benefit with clipping near zero, but I haven't gone too far beyond the changes to align it to GEOS.

That can be another PR, sometime.

If there's no more changes required, I can merge this.

[mwtoews]

Minor GEOS renaming done here: https://github.com/libgeos/geos/commit/dcde8ad8a15eabdafd3a7c3ef78d6cf20cf800de

And yes, this PR is now set to merge.