Remove empty geometries before marshaling.

[ccma14]

Description

This PR fixes an issue that causes tilenol to crash if a layer returned contains an empty geometry (stack trace below).

The issue is that tilenol attempts to Marshal layers into a gzipped MVT before storing it in the optional cache. However, Marshaling a Layer into a gzipped MVT via mvt.MarshalGzipped causes a panic if an empty geometry is encountered.

This one-line fix adds a call to Layer.removeEmpty after retrieving it from the source, but before marshaling it for storage in the cache. This function also removes empty geometries (not just too small ones), and hence the issue is avoided.

Testing Done

• Confirmed that I can no longer reproduce the crash with the patch.
• Testing shows that MVTs served by tilenol seem otherwise unaffected.

panic: geometry type not supported: <nil>

goroutine 258 [running]:
github.com/paulmach/orb/encoding/mvt.encodeGeometry(0x0, 0x0, 0x0, 0x2, 0x2, 0x0, 0x0, 0x0)
	/go/pkg/mod/github.com/paulmach/[email protected]/encoding/mvt/geometry.go:90 +0x1d37
github.com/paulmach/orb/encoding/mvt.Marshal(0xc0005c7cb8, 0x1, 0x1, 0x0, 0x7, 0xc0001e8130, 0xc0003880f0, 0xc000442e40)
	/go/pkg/mod/github.com/paulmach/[email protected]/encoding/mvt/marshal.go:60 +0x126
github.com/paulmach/orb/encoding/mvt.MarshalGzipped(0xc0005c7cb8, 0x1, 0x1, 0xc0001e8130, 0x7, 0x0, 0x0, 0xe)
	/go/pkg/mod/github.com/paulmach/[email protected]/encoding/mvt/marshal.go:21 +0x5a
github.com/stationa/tilenol.(*Server).getLayerData(0xc000183bf0, 0xdfb080, 0xc0002eeb00, 0xc0001e8130, 0x7, 0x0, 0x0, 0xe, 0x0, 0x1, ...)
	/go/src/github.com/stationa/tilenol/server.go:266 +0x345
github.com/stationa/tilenol.(*Server).getVectorTile.func2(0x0, 0x0)
	/go/src/github.com/stationa/tilenol/server.go:321 +0x308
golang.org/x/sync/errgroup.(*Group).Go.func1(0xc000388690, 0xc0003864d0)
	/go/pkg/mod/golang.org/x/[email protected]/errgroup/errgroup.go:57 +0x59
created by golang.org/x/sync/errgroup.(*Group).Go
	/go/pkg/mod/golang.org/x/[email protected]/errgroup/errgroup.go:54 +0x66

[jerluc]

Thanks @ccma14 this looks indeed like it should trim out empty geometries. I have a few questions:

• Do you know what units the lineLimit and areaLimit parameters are? I'm just wondering if we need to lower the threshold at all.
• Looking into the implementation for removeEmpty, I can see it basically runs through all of the features in a layer, computes length/area, and omits anything that doesn't meet the thresholds given. Do you think this might have any major performance implications? It's not completely the same computationally, but it reminds me a bit of shape simplification which was removed by default in fa73964
• Do you have an MVT or any other minimally-reproducible example we can check in as an extra test to protect against regression?

[ccma14]

To address the concerns in terms of run time overhead and side effects from calling removeEmpty, I've modified my patch such that we now use a small function that just filters out empty Geometries instead.

[jerluc]

Thanks @ccma14, I just need to quickly make format and get this merged down!