Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

encoding/mvt: skip encoding of features will nil geometry #141

Merged
merged 1 commit into from
Jan 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion encoding/mvt/clip.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]
}
26 changes: 24 additions & 2 deletions encoding/mvt/clip_test.go
Original file line number Diff line number Diff line change
@@ -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) {
Expand Down Expand Up @@ -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)
}
}
5 changes: 5 additions & 0 deletions encoding/mvt/marshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
Expand Down Expand Up @@ -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)
Expand Down
33 changes: 33 additions & 0 deletions encoding/mvt/marshal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading