From 79447a1405a03259cb33afecebb6db4bc9cb6107 Mon Sep 17 00:00:00 2001
From: Paul Mach <paul@mach.org>
Date: Mon, 8 Jan 2024 16:20:42 -0800
Subject: [PATCH] encoding/mvt: skip encoding of features will nil geometry

---
 encoding/mvt/clip.go         | 11 ++++++++++-
 encoding/mvt/clip_test.go    | 26 ++++++++++++++++++++++++--
 encoding/mvt/marshal.go      |  5 +++++
 encoding/mvt/marshal_test.go | 33 +++++++++++++++++++++++++++++++++
 4 files changed, 72 insertions(+), 3 deletions(-)

diff --git a/encoding/mvt/clip.go b/encoding/mvt/clip.go
index 2e8cf62..3853d61 100644
--- a/encoding/mvt/clip.go
+++ b/encoding/mvt/clip.go
@@ -22,9 +22,18 @@ func (ls Layers) Clip(box orb.Bound) {
 }
 
 // Clip will clip all geometries in this layer to the given bounds.
+// Will remove features that clip to an empty geometry, modifies the
+// layer.Features slice in place.
 func (l *Layer) Clip(box orb.Bound) {
+	at := 0
 	for _, f := range l.Features {
 		g := clip.Geometry(box, f.Geometry)
-		f.Geometry = g
+		if g != nil {
+			f.Geometry = g
+			l.Features[at] = f
+			at++
+		}
 	}
+
+	l.Features = l.Features[:at]
 }
diff --git a/encoding/mvt/clip_test.go b/encoding/mvt/clip_test.go
index 442070e..0fa4bce 100644
--- a/encoding/mvt/clip_test.go
+++ b/encoding/mvt/clip_test.go
@@ -1,10 +1,11 @@
 package mvt
 
 import (
-	"github.com/paulmach/orb"
-	"github.com/paulmach/orb/geojson"
 	"reflect"
 	"testing"
+
+	"github.com/paulmach/orb"
+	"github.com/paulmach/orb/geojson"
 )
 
 func TestLayersClip(t *testing.T) {
@@ -56,3 +57,24 @@ func TestLayersClip(t *testing.T) {
 		})
 	}
 }
+
+func TestLayerClip_empty(t *testing.T) {
+	layer := &Layer{
+		Features: []*geojson.Feature{
+			geojson.NewFeature(orb.Polygon{{
+				{-1, 1}, {0, 1}, {1, 1}, {1, 5}, {1, -5},
+			}}),
+			geojson.NewFeature(orb.LineString{{55, 0}, {66, 0}}),
+		},
+	}
+
+	layer.Clip(orb.Bound{Min: orb.Point{50, -10}, Max: orb.Point{70, 10}})
+
+	if v := len(layer.Features); v != 1 {
+		t.Errorf("incorrect number of features: %d", v)
+	}
+
+	if v := layer.Features[0].Geometry.GeoJSONType(); v != "LineString" {
+		t.Errorf("kept the wrong geometry: %v", layer.Features[0].Geometry)
+	}
+}
diff --git a/encoding/mvt/marshal.go b/encoding/mvt/marshal.go
index 7cabef6..a54dcbe 100644
--- a/encoding/mvt/marshal.go
+++ b/encoding/mvt/marshal.go
@@ -40,6 +40,7 @@ func MarshalGzipped(layers Layers) ([]byte, error) {
 }
 
 // Marshal will take a set of layers and encode them into a Mapbox Vector Tile format.
+// Features that have a nil geometry, for some reason, will be skipped and not included.
 func Marshal(layers Layers) ([]byte, error) {
 	vt := &vectortile.Tile{
 		Layers: make([]*vectortile.Tile_Layer, 0, len(layers)),
@@ -73,6 +74,10 @@ func Marshal(layers Layers) ([]byte, error) {
 }
 
 func addFeature(layer *vectortile.Tile_Layer, kve *keyValueEncoder, f *geojson.Feature) error {
+	if f.Geometry == nil {
+		return nil
+	}
+
 	if f.Geometry.GeoJSONType() == "GeometryCollection" {
 		for _, g := range f.Geometry.(orb.Collection) {
 			return addSingleGeometryFeature(layer, kve, g, f.Properties, f.ID)
diff --git a/encoding/mvt/marshal_test.go b/encoding/mvt/marshal_test.go
index 2ad2d44..8d50b41 100644
--- a/encoding/mvt/marshal_test.go
+++ b/encoding/mvt/marshal_test.go
@@ -138,6 +138,39 @@ func TestMarshalUnmarshalForGeometryCollection(t *testing.T) {
 	}
 }
 
+func TestMarshal_nilGeometry(t *testing.T) {
+	// should handle nil geometry, possibly caused by clipping
+	// or other issues.
+
+	fc := geojson.NewFeatureCollection()
+	fc.Append(geojson.NewFeature(nil))
+
+	layers := Layers{NewLayer("roads", fc)}
+
+	// project to the tile coords
+	tile := maptile.New(0, 0, 15)
+	layers.ProjectToTile(tile)
+
+	// marshal
+	encoded, err := MarshalGzipped(layers)
+	if err != nil {
+		t.Fatalf("marshal error: %v", err)
+	}
+
+	// unmarshal
+	decoded, err := UnmarshalGzipped(encoded)
+	if err != nil {
+		t.Fatalf("unmarshal error: %v", err)
+	}
+
+	// project back
+	decoded.ProjectToWGS84(tile)
+
+	if v := len(decoded[0].Features); v != 0 {
+		t.Errorf("should have have any features, has %v", v)
+	}
+}
+
 func TestMarshalUnmarshal(t *testing.T) {
 	cases := []struct {
 		name string