GeoJSON plugin sometimes reversing order of features

[danpat]

In some circumstances, it seems that the geojson datasource plugin is not maintaining feature ordering.

I am drawing LineString features from a GeoJSON file on a map, colouring them based on an "age" property. The features are listed in chronological order in the GeoJSON file. I rely on the latter features being painted over the top of the earlier features to composite the final image.

If I load the GeoJSON with the ogr plugin, I get the following correct rendering:

However, if I use the geojson datasource plugin, I sometimes see this, for the same data/stylesheets:

I'm having difficulty coming up with a minimal reproduction case, but the only change between the two above it the datasource type, the datafile and stylesheet remained the same.

In all cases, the "green" data is listed later in the GeoJSON. The second image hints that the ordering of the features is somewhat randomized, it does not appear to be strictly reversed.

The above results were achieved with tilelive-mapnik using mapnik@3.4.10.

[danpat]

More info: it looks like it's a combination of geojson and cache_features that causes this mis-ordering.

The following causes the problem:

<Layer name="grooming" srs="+init=epsg:4326">
    <StyleName>grooming</StyleName>
    <Datasource>
       <Parameter name="type">geojson</Parameter>
       <Parameter name="file">grooming.geojson</Parameter>
       <Parameter name="cache_features">true</Parameter>
    </Datasource>
</Layer>

flipping cache_features makes it go away.

[artemp]

@danpat - good catch! aee0149 should fix the issue, could you verify with your dataset and I'll close this issue.

[springmeyer]

@danpat can you help reduce a regression testcase? The idea would be to craft an xml+geojson that triggered the issue, check the xml into https://github.com/mapnik/test-data-visual, and the geojson into https://github.com/mapnik/test-data, then ensure it is fixed by @artemp's commit. Once you have that confirmation then we'ed update mapnik master to point at the new test data by updating the git submodules.

[artemp]

@danpat @springmeyer - this is asking for a unit test not a visual one!! I'm working on it

[artemp]

unit test is in place ^, closing

[springmeyer]

Excellent!. Confirmed that if I revert the bugfix (aee0149) I get a failure as expected now:

test/unit/datasource/geojson.cpp:621: FAILED:
  REQUIRE( val.get<mapnik::value_integer>() == ++count )
with expansion:
  19 == 1