[Bug] GeoJsonLayer with binary data does not draw closed polygons

[aethr]

Description

When using GeoJsonLayer to draw Polygons from binary data, closed shapes are not joined on the last position. For wide stroke widths, this causes a notched corner where line caps from the first and last vertex meet at different angles.

When the same polygon is provided via json data, the polygon is closed with no line caps.

Possible cause

The createLayerPropsFromBinary method in the GeoJsonLayer module explicitly sets _pathType = 'open' as the final part of parsing stroke geometry to pass to PathLayer. Unfortunately, because of how this is done, I couldn't figure out how to override it in my calling code.

https://github.com/visgl/deck.gl/blob/master/modules/layers/src/geojson-layer/geojson-layer-props.ts#L138

I'm not sure why this is set, although I'm sure there's a good reason. This line appears in the initial commit to support binary data: f11ee4f#diff-d1664e13173d9407c92e7ee39e9ff8aae2e325ab4c3a34cc1b091f8c44f7c4d4.

In the same commit a note in the docs was left which may be related?

- In binary format, there are some rendering issues with polygons or multipolygons which contain holes (the holes won't appear as expected), we're working to fix it for the next release.

The description of PathLayer's _pathType attribute (https://deck.gl/docs/api-reference/layers/path-layer#_pathtype) suggests that leaving this unset may cause better performance, so it would be great to be able to enable/disable this depending on your individual use case if you trust the quality of your data.

Just wanted to add a small note to say that this library is really amazing and has allowed me to accomplish so much in such a short time, all your hard work is truly appreciated!

Flavors

• Script tag
• React
• Python/Jupyter notebook
• MapboxOverlay
• GoogleMapsOverlay
• CartoLayer
• ArcGIS

Expected Behavior

Stroke on closed polygons should have corners drawn at the final position regardless of how the data is provided.

Steps to Reproduce

https://codepen.io/aethrXor/pen/MWMLvPQ?editors=0010

This repro defines a single GeoJson Polygon feature, and uses geojsonToBinary to convert it to binary. Two layers are created, one with the json data, one with the binary. Toggling visibility between the two layers, you can see a notch in the bottom right corner when the binary layer is visible.

Environment

• Framework version: [email protected]
• Browser: Chrome
• OS: Linux

Logs

No response

[aethr]

I assume that setting _pathType: 'open' for the PathLayer is done to provide safety for GeoJSON that may not be well formed or GeoJSON is loaded from unknown sources. It would be nice to at least be able to override this default when working with data that is more predictable.

If I put a PR together which made this a default prop value for the LineLayer sublayer but allowed it to be provided as a linePathType prop to GeoJSONLayer, would that be ok?

[aethr]

My thought would be to add a prop to GeoJsonLayer called linePathType, which is forwarded to the internal PathLayer _pathType prop, with a default value of 'open' to match the current binary implementation.

However, looking at the current implementation, the default value of 'open' only applies to GeoJsonLayer when data is a binary type. If we add a new prop with a default value, that would be the default regardless of whether data is supplied as json or binary. In that case, the default should probably be null to match the default behaviour of GeoJsonLayer.

This would mean it's a breaking change one way or the other, possibly mitigated by the fact that it's probably a bug to have a different value for this PathLayer prop just because of how the data is provided.

[felixpalmer]

How you tried subLayerProps?

_subLayerProps: {
  'linestrings': {
    _pathType: 'loop'
  }
}

Not sure if this will work with the binary data though

[aethr]

@felixpalmer I did try that without success, but it was my first attempt at using _subLayerProps, so that's not conclusive.

However, I believe that because of how the props are passed to the PathLayer here: https://github.com/visgl/deck.gl/blob/master/modules/layers/src/geojson-layer/geojson-layer.ts#L503-L510, props set in layerProps.polygonsOutline will be passed last, and so will take precedence over any props provided earlier.

Because of how polygonsOutline._pathType is set for binary data in https://github.com/visgl/deck.gl/blob/master/modules/layers/src/geojson-layer/geojson-layer-props.ts#L138, it makes it very hard to override.

My opinion is that even if this behaviour can be overridden, it's still probably a bug in GeoJsonLayer that you get different behaviour when you pass identical data via binary.

[aethr]

I tried adding this to the reproduction and it didn't have any effect, providing _pathType: null or _pathType: 'loop' via _subLayerProps:

        _subLayerProps: {
          'polygons-stroke': {
            _pathType: null,
          },
        },