OSGeo / PROJ-JNI

Java Native Interface for PROJ
https://osgeo.github.io/PROJ-JNI/
MIT License
23 stars 15 forks source link

Each subsequent (CoordinateOperation) transform will fail after exception #61

Closed msimons closed 1 year ago

msimons commented 2 years ago

If a TransformException occurs when executing the transform method, each subsequent call results in the same error. It seems that certain state is not being cleaned up properly. In this situation we are now forced to create a new instance of CoordinateOperation. This is of course undesirable in the context of performance.

The issue can be reproduced with the code below (PROJ9 & latest PROJ-JNI code).

// WKT transformation for ETRS89 height -> Dutch NAP height
// The transformation will only succeed for points within the RD bounding box else an TransformException "Coordinate to transform falls outside grid" will be thrown by PROJ
CoordinateOperation operation = (CoordinateOperation) Proj.createFromUserInput("COORDINATEOPERATION[\"ETRS89 to ETRS89 + NAP height (2)\",VERSION[\"NSGI-Nld 2018\"],SOURCECRS[GEOGCRS[\"ETRS89\",ENSEMBLE[\"European Terrestrial Reference System 1989 ensemble\", MEMBER[\"European Terrestrial Reference Frame 1989\", ID[\"EPSG\",1178]], MEMBER[\"European Terrestrial Reference Frame 1990\", ID[\"EPSG\",1179]], MEMBER[\"European Terrestrial Reference Frame 1991\", ID[\"EPSG\",1180]], MEMBER[\"European Terrestrial Reference Frame 1992\", ID[\"EPSG\",1181]], MEMBER[\"European Terrestrial Reference Frame 1993\", ID[\"EPSG\",1182]], MEMBER[\"European Terrestrial Reference Frame 1994\", ID[\"EPSG\",1183]], MEMBER[\"European Terrestrial Reference Frame 1996\", ID[\"EPSG\",1184]], MEMBER[\"European Terrestrial Reference Frame 1997\", ID[\"EPSG\",1185]], MEMBER[\"European Terrestrial Reference Frame 2000\", ID[\"EPSG\",1186]], MEMBER[\"European Terrestrial Reference Frame 2005\", ID[\"EPSG\",1204]], MEMBER[\"European Terrestrial Reference Frame 2014\", ID[\"EPSG\",1206]], ELLIPSOID[\"GRS 1980\",6378137,298.257222101,LENGTHUNIT[\"metre\",1,ID[\"EPSG\",9001]],ID[\"EPSG\",7019]], ENSEMBLEACCURACY[0.1],ID[\"EPSG\",6258]],CS[ellipsoidal,3,ID[\"EPSG\",6423]],AXIS[\"latitude (Lat)\",north,ANGLEUNIT[\"degree\",0.0174532925199433,ID[\"EPSG\",9102]]],AXIS[\"longitude (Lon)\",east,ANGLEUNIT[\"degree\",0.0174532925199433,ID[\"EPSG\",9102]]],AXIS[\"Ellipsoidal height (h)\",up,LENGTHUNIT[\"metre\",1,ID[\"EPSG\",9001]]],ID[\"EPSG\",4937]]],TARGETCRS[COMPOUNDCRS[\"ETRS89 + NAP height\",GEOGCRS[\"ETRS89\",ENSEMBLE[\"European Terrestrial Reference System 1989 ensemble\", MEMBER[\"European Terrestrial Reference Frame 1989\", ID[\"EPSG\",1178]], MEMBER[\"European Terrestrial Reference Frame 1990\", ID[\"EPSG\",1179]], MEMBER[\"European Terrestrial Reference Frame 1991\", ID[\"EPSG\",1180]], MEMBER[\"European Terrestrial Reference Frame 1992\", ID[\"EPSG\",1181]], MEMBER[\"European Terrestrial Reference Frame 1993\", ID[\"EPSG\",1182]], MEMBER[\"European Terrestrial Reference Frame 1994\", ID[\"EPSG\",1183]], MEMBER[\"European Terrestrial Reference Frame 1996\", ID[\"EPSG\",1184]], MEMBER[\"European Terrestrial Reference Frame 1997\", ID[\"EPSG\",1185]], MEMBER[\"European Terrestrial Reference Frame 2000\", ID[\"EPSG\",1186]], MEMBER[\"European Terrestrial Reference Frame 2005\", ID[\"EPSG\",1204]], MEMBER[\"European Terrestrial Reference Frame 2014\", ID[\"EPSG\",1206]], ELLIPSOID[\"GRS 1980\",6378137,298.257222101,LENGTHUNIT[\"metre\",1,ID[\"EPSG\",9001]],ID[\"EPSG\",7019]], ENSEMBLEACCURACY[0.1],ID[\"EPSG\",6258]],CS[ellipsoidal,2,ID[\"EPSG\",6422]],AXIS[\"latitude (Lat)\",north],AXIS[\"longitude (Lon)\",east],ANGLEUNIT[\"degree\",0.0174532925199433,ID[\"EPSG\",9102]],ID[\"EPSG\",4258]],VERTCRS[\"NAP height\",VDATUM[\"Normaal Amsterdams Peil\",ID[\"EPSG\",5109]],CS[vertical,1,ID[\"EPSG\",6499]],AXIS[\"Gravity-related height (H)\",up],LENGTHUNIT[\"metre\",1,ID[\"EPSG\",9001]],ID[\"EPSG\",5709]],ID[\"EPSG\",9286]]],METHOD[\"Geog3D to Geog2D+GravityRelatedHeight (gtx)\",ID[\"EPSG\",1088]],PARAMETERFILE[\"Geoid (height correction) model file\",\"nlgeo2018.gtx\",ID[\"EPSG\",8666]],PARAMETER[\"EPSG code for Interpolation CRS\",4258,ID[\"EPSG\",1048]],OPERATIONACCURACY[0.001],ID[\"EPSG\",9597]]");

// Coordinate within RD bounding box
double[] coordinateInside = {
    53.106038193, 5.255859149,  345.4981
};

// first transform 
operation.getMathTransform().transform(
    coordinateInside, 0,
    coordinateInside, 0,
    1);

// Coordinate outside RD bounding box
double[] coordinatesOutside = {
    45.25635676, -2.00693553, 208.6062
};

// second transform results in TransformException 'Coordinate to transform falls outside grid' as expected
try {
  operation.getMathTransform().transform(
      coordinatesOutside, 0,
      coordinatesOutside, 0,
      1);
} catch (TransformException exception) {
  log.error("Expected transformException!",exception);
}

// Any subsequent transformation after the second transformation on the same `CoordinateOperation` instance is failing unexpectedly
operation.getMathTransform().transform(
    coordinateInside, 0,
    coordinateInside, 0,
    1);
desruisseaux commented 2 years ago

A quick report on current status. I could not yet reproduce the issue with PROJ 8.2.1 (the version provided by my Fedora Linux distribution) because that version of PROJ seems to return a NULL object for that coordinate operation. I previously thought that NULL objects would be returned by the C/C++ API only when running out of memory, but there is apparently other possible reasons. The Java exception has been changed in #62 for reflecting that new assumption.

I'm lazily waiting for the Fedora distribution to upgrade to PROJ 9 before to test again. If it takes too long, I may try to compile locally later.

desruisseaux commented 1 year ago

I finally could reproduce. No state were kept in Java code or in JNI bindings, we only have a pointer to PROJ PJ object. We would need to test directly in C/C++ to confirm if this is a PROJ bug or if we missed something else. Or maybe it is part of PROJ contract to not use a PJ object any more after an error. The fix applied in Java code is to unconditionally discard the native PJ object after an error instead of recycling it for next operations.

desruisseaux commented 1 year ago

Actually it seems that a better fix was already applied in a fork: https://github.com/interactive-instruments/PROJ-JNI/commit/c3b7e3b587ca19b307a6b6f47e441d8fc0f0a03e

Waiting for #63 to be fixed before to try the better fix.

desruisseaux commented 1 year ago

Reverted previous fix and use proj_errno_reset(pj) instead.