Esri / geometry-api-java

The Esri Geometry API for Java enables developers to write custom applications for analysis of spatial data. This API is used in the Esri GIS Tools for Hadoop and other 3rd-party data processing solutions.
Apache License 2.0
694 stars 260 forks source link

OperatorSimplifyOGC leads to "Index was outside the bounds of the array" in StridedIndexTypeCollection #284

Closed DaveInCaz closed 2 years ago

DaveInCaz commented 3 years ago

I've been using the project https://github.com/EchoParkLabs/geometry-api-cs which seems to be derived from here. I ran into this error "Index was outside the bounds of the array."

This originated from this call:

OperatorSimplifyOGC.Local().Execute(polygon, null, true, null);

and produced the following stack trace:

   at com.epl.geometry.StridedIndexTypeCollection.GetField(Int32 element, Int32 field)
   at com.epl.geometry.TopoGraph.SetChainParent_(Int32 chain, Int32 parentChain)
   at com.epl.geometry.TopoGraph.PlaneSweepParentagePropagateParentage_(Treap aet, Int32 treeNode, Int32 inputMode)
   at com.epl.geometry.TopoGraph.PlaneSweepParentage_(Int32 inputMode, ProgressTracker progress_tracker)
   at com.epl.geometry.TopoGraph.SetEditShapeImpl_(EditShape shape, Int32 inputMode, AttributeStreamOfInt32 editShapeGeometries, ProgressTracker progress_tracker, Boolean bBuildChains)
   at com.epl.geometry.TopoGraph.SetAndSimplifyEditShapeAlternate(EditShape shape, Int32 geometry, ProgressTracker progressTracker)
   at com.epl.geometry.TopologicalOperations.PlanarSimplify(EditShape shape, Int32 geom, Double tolerance, Boolean b_use_winding_rule_for_polygons, Boolean dirty_result, ProgressTracker progress_tracker)
   at com.epl.geometry.TopologicalOperations.PlanarSimplifyImpl_(MultiVertexGeometry input_geom, Double tolerance, Boolean b_use_winding_rule_for_polygons, Boolean dirty_result, ProgressTracker progress_tracker)
   at com.epl.geometry.TopologicalOperations.SimplifyOGC(MultiVertexGeometry input_geom, Double tolerance, Boolean dirty_result, ProgressTracker progress_tracker)
   at com.epl.geometry.OperatorSimplifyLocalHelper.SimplifyOGC(Geometry geometry, SpatialReference spatialReference, Boolean bForce, ProgressTracker progressTracker)
   at com.epl.geometry.OperatorSimplifyCursorOGC.Simplify(Geometry geometry)
   at com.epl.geometry.OperatorSimplifyCursorOGC.Next()
   at com.epl.geometry.OperatorSimplifyLocalOGC.Execute(Geometry geom, SpatialReference spatialRef, Boolean bForceSimplify, ProgressTracker progressTracker)

I can consistently reproduce this locally within my application.

Digging around in the debugger I think I found what constitutes the X,Y coordinates of the polygon being operated on:

  Name Value Type
m_buffer {double[8]} double[]
  [0] 136239 double
  [1] 3047 double
  [2] 98207 double
  [3] 3047.0000000000005 double
  [4] 98207 double
  [5] 9448 double
  [6] 136239 double
  [7] 9448 double

That was from: polygon -> m_impl -> m_vertexAttributes[0] -> m_buffer

I guess this is the individual X,Y coordinates in series, making up 4 total points. (polygon.GetPointCount() returns 4). If there are any other relevant details of that polygon I can certainly add them.


This is the original definition of the function throwing the error:

int getField(int element, int field) {
    assert(m_buffer[element >> m_blockPower][(element & m_blockMask) + 1] != -0x7eadbeed);
    return m_buffer[element >> m_blockPower][(element & m_blockMask)    + field];
}

and this is the C# equivalent in the other project:

internal int GetField(int element, int field)
{
    System.Diagnostics.Debug.Assert((m_buffer[element >> m_blockPower][(element & m_blockMask) + 1] != -unchecked((int)(0x7eadbeed))));
    return m_buffer[element >> m_blockPower][(element & m_blockMask) + field];
}
stolstov commented 3 years ago

@DaveInCaz Could you share the polygon that causes this? You could save it to a json string or a file.

DaveInCaz commented 3 years ago

Given a polygon object, is there is a simple method to write it out to a JSON string? (Sorry but I have never done that before).

My manual transcription of the points is the following:

X, Y
136239, 3047
98207, 3047.0000000000005
98207, 9448
136239, 9448
stolstov commented 3 years ago

@DaveInCaz To save the polygon you could use OperatorFactoryLocal.saveJSONToTextFileDbg(String file_name, Geometry geometry, SpatialReference spatial_ref). spatial_ref can be null. https://github.com/Esri/geometry-api-java/blob/master/src/main/java/com/esri/core/geometry/OperatorFactoryLocal.java#L160

However, I've missed that you have provided the coordinates of the polygon in the original issue, so json is not needed right now. I'll try to reproduce this issue in Java.

stolstov commented 3 years ago

@DaveInCaz Here is the test that I've made from the data you've provided. It does not reproduce the crash. Can you try porting this test to C# and check if it reproduces the issue?

    @Test
    public void testCrash() {
        MapGeometry mg = OperatorImportFromJson.local().execute(Geometry.Type.Unknown, 
                "{\"rings\":[[[136239, 3047],[98207, 3047.0000000000005],[98207, 9448],[136239, 9448],[136239, 3047]]]}");

        Geometry simpleResult = OperatorSimplifyOGC.local().execute(mg.getGeometry(), null, true, null);
        //String str = OperatorExportToJson.local().execute(null, simpleResult);
        String baseStr = "{\"rings\":[[[136239,3047],[98207,3047.0000000000005],[98207,9448],[136239,9448],[136239,3047]]]}";
        MapGeometry baseGeom = OperatorImportFromJson.local().execute(Geometry.Type.Unknown, baseStr);
        boolean res = baseGeom.getGeometry().equals(simpleResult);
        assertTrue(res);
    }
stolstov commented 3 years ago

updated the test a little...

DaveInCaz commented 3 years ago

It seems that the JSON import class does not exist in the C# library - I checked both the compiled Nuget package and also the source download from GitHub. (There are actually JSON classes in the source but they are not included in the project. It looks like that might rely on Java / Jackson resources which aren't available to C#?)

Is there a convenient alternative way to formulate the test program without using the JSON capabilities?

stolstov commented 3 years ago

@DaveInCaz

        Polygon poly = new Polygon();
        poly.startPath(136239, 3047);
        poly.lineTo(98207, 3047.0000000000005);
        poly.lineTo(98207, 9448);
        poly.lineTo(136239, 9448);
DaveInCaz commented 3 years ago

Thanks! (and I realize now, I should have though of that myself...)

Running the following test program, the end result is that they are EQUAL:

   class Program
    {
        static void Main(string[] args)
        {
            TestCrash();
        }

        public static void TestCrash()
        {
            Polygon poly = new Polygon();
            poly.StartPath(136239, 3047);
            poly.LineTo(98207, 3047.0000000000005);
            poly.LineTo(98207, 9448);
            poly.LineTo(136239, 9448);

            Geometry simpleResult = OperatorSimplifyOGC.Local().Execute(poly, null, true, null);

            bool res = poly.Equals(simpleResult);
            Debug.Assert(res);
        }
    }

It does not reproduce the same error. Did my C# version capture the same intent of the Java example?

Do you have any other suggestions how to debug this? (Thank you for your help!)

stolstov commented 3 years ago

@DaveInCaz It looks the same. If you can consistently reproduce the issue, try capturing the content of the polygon before the simplify call and add it to the unit test.

DaveInCaz commented 3 years ago

Incidentally, while looking into the JSON code I discovered the WKT export, which is also a useful way just to dump the contents in a readable format. Doing that in the debugger on the polygon object which produces the original error I get this:

EPL.OperatorExportToWkt.Local().Execute(0, polygon, null)
"MULTIPOLYGON (((136239 3047, 136239 9448, 98207 9448, 98207 3047.0000000000005, 136239 3047)))"

So it looks like I had the order wrong in my data above, and also the first/last point is repeated.

Running the test again like this:

        public static void TestCrash()
        {
            Polygon poly = new Polygon();
            poly.StartPath(136239, 3047);
            poly.LineTo(136239, 9448);
            poly.LineTo(98207, 9448);
            poly.LineTo(98207, 3047.0000000000005d);
            poly.LineTo(136239, 3047);

            Geometry simpleResult = OperatorSimplifyOGC.Local().Execute(poly, null, true, null);

            bool res = poly.Equals(simpleResult);
            Debug.Assert(res);
        }

There still is no crash, but now the Equalstest returns FALSE. Is this significant?

stolstov commented 3 years ago

@DaveInCaz WKT export maybe reversing the order of the vertices, it is by design.

DaveInCaz commented 3 years ago

I think I found a solution that will work for me...

I had been using the package https://www.nuget.org/packages/EchoParkLabs.Computational.Geometry/2.0.0-alpha68 to provide the C# library. However I updated to the latest code from GitHub in its place, and the error disappeared.

I assume there must be some difference even though this is not evident in the GitHub history. History seemed to show that only some cosmetic changes to comments were made after the point where it looks like the Nuget package was created; but there must have been something else.

Ideally that C# project would update the Nuget to include the latest code; but I'm not sure how active it is... it looks like issues posted to that project aren't looked at.

Thanks again for the help here, anyway!

DaveInCaz commented 2 years ago

@stolstov I think I discovered what the real issue is here. Some of the debug code may be causing a side effect which prevents the error. When that debug code is compiled out, the error can occur.

The reason that local compilation appeared to fix the issue was because the Nuget was probably compiled with optimizations. My own project was at that time only being compiled in Debug mode. But more recently we started building in Release which includes optimizations by default, and the problem came back.

Recompiling geometry-api-cs in Release but with optimizations OFF - the error does NOT occur.  So I am doing that now as a workaround but it would be ideal to include the optimizations of course.

Seems like there could be some debug code that actually is critical?

FYI I also updated the library to .NET 4.7.2 but that didn't affect this issue at all.