search

dr-jts / jts

The JTS Topology Suite is a Java library for creating and manipulating vector geometry.
Other
22 stars 6 forks source link

New Geometry has old Precision Model #8

Open pramsey opened 4 years ago

pramsey commented 4 years ago

https://github.com/dr-jts/jts/blob/cb2506e0a39f30fb581c424678c9be8eca376746/modules/core/src/main/java/org/locationtech/jts/operation/overlayng/OverlayNG.java#L348

Why is the geomFact, which is used to create the resultant geometry, a reference to the factory from geom0 instead of a new construction based on pm?

pramsey commented 4 years ago

In adding the NG precision reduction to the CAPI in GEOS, I had to add an OverlayNG constructor that took a GeometryFactory, in order to obey GEOS ownership rules. Relevant changes are here: https://github.com/pramsey/geos/commit/678babdbe772da2c2b2cd2d240a65f84ff0c4311

dr-jts commented 4 years ago

Because the contract of OverlayNG doesn't include that the precision model will be changed. It follows a new (and hopefully simpler) pattern where the overlay can be carried out in fixed precision but on FLOATING inputs (and outputs).

On Mon, Aug 24, 2020 at 1:03 PM Paul Ramsey notifications@github.com wrote:

https://github.com/dr-jts/jts/blob/cb2506e0a39f30fb581c424678c9be8eca376746/modules/core/src/main/java/org/locationtech/jts/operation/overlayng/OverlayNG.java#L348

Why is the geomFact, which is used to create the resultant geometry, a reference to the factory from geom0 instead of a new construction based on pm?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/dr-jts/jts/issues/8, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA25SXOZDFMUM6UMGW352GLSCLBSXANCNFSM4QJ3V2TQ .

pramsey commented 4 years ago

I can grok that. For precision reduction though, I think it makes sense to build the new geometry with a precisionModel that matches the reduced geometry? Or maybe not? The assumption in the existing GEOS code is that the new geometry gets a new coarse precisionModel, so I aped that. However, reducing precision while retaining a FLOAT model is also a valid use case. ... Reasoning about precision is hard!

dr-jts commented 4 years ago

For precision reduction though, I think it makes sense to build the new geometry with a precisionModel that matches the reduced geometry? Or maybe not? The assumption in the existing GEOS code is that the new geometry gets a new coarse precisionModel, so I aped that. However, reducing precision while retaining a FLOAT model is also a valid use case. ... Reasoning about precision is hard!

I think there's an argument to be made for offering both options. But not sure what the best/simplest API is for doing this. Usually in this kind of design situation I like to factor out the two functions into separate API calls, to provide generality.

JTS provides the GeometryPrecisionReducer.setChangePrecisionModel method to allow the user to choose the behaviour wanted.