Skip to content

Commit

Permalink
Replaced s2 contains point methods with go-geom (hypermodeinc#5023)
Browse files Browse the repository at this point in the history
Leads to significantly higher performance.

```
goos: linux
goarch: amd64
BenchmarkMatchesFilterContainsPoint-8               3902            312904 ns/op
BenchmarkMatchesFilterContainsPoint_Before-8          31          35054224 ns/op
PASS
ok      command-line-arguments  2.636s
```

From my testing while working on a project of my own I found that while using s2 types is very fast, converting to them is very slow, so if you're converting each time it's much faster to just use the go-geom types directly. This PR just changes this for contains point queries.
  • Loading branch information
sams96 authored Mar 30, 2020
1 parent ff1a983 commit ff69bd2
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 10 deletions.
32 changes: 22 additions & 10 deletions types/geofilter.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/dgraph-io/dgraph/protos/pb"
"github.com/dgraph-io/dgraph/x"
"github.com/pkg/errors"
"github.com/twpayne/go-geom/xy"
)

// QueryType indicates the type of geo query.
Expand Down Expand Up @@ -311,13 +312,14 @@ func (q GeoQueryData) contains(g geom.T) bool {
x.AssertTruef(q.pt != nil || len(q.loops) > 0, "At least a point or loop should be defined.")
switch v := g.(type) {
case *geom.Polygon:
if q.pt != nil {
return polygonContainsCoord(v, q.pt)
}

s2loop, err := loopFromPolygon(v)
if err != nil {
return false
}
if q.pt != nil {
return s2loop.ContainsPoint(*q.pt)
}

// Input could be a multipolygon, in which q.loops would have more than 1 loop. Each loop
// in the query should be part of the s2loop.
Expand All @@ -329,16 +331,13 @@ func (q GeoQueryData) contains(g geom.T) bool {
return true
case *geom.MultiPolygon:
if q.pt != nil {
for i := 0; i < v.NumPolygons(); i++ {
p := v.Polygon(i)
s2loop, err := loopFromPolygon(p)
if err != nil {
return false
}
if s2loop.ContainsPoint(*q.pt) {
for j := 0; j < v.NumPolygons(); j++ {
if polygonContainsCoord(v.Polygon(j), q.pt) {
return true
}
}

return false
}

if len(q.loops) > 0 {
Expand All @@ -358,6 +357,19 @@ func (q GeoQueryData) contains(g geom.T) bool {
}
}

func polygonContainsCoord(v *geom.Polygon, pt *s2.Point) bool {
for i := 0; i < v.NumLinearRings(); i++ {
r := v.LinearRing(i)
ll := s2.LatLngFromPoint(*pt)
p := []float64{ll.Lng.Degrees(), ll.Lat.Degrees()}
if xy.IsPointInRing(r.Layout(), p, r.FlatCoords()) {
return true
}
}

return false
}

// returns true if the geometry represented by uid/attr intersects the given loop or point
func (q GeoQueryData) intersects(g geom.T) bool {
x.AssertTruef(len(q.loops) > 0, "Loop should be defined for intersects.")
Expand Down
20 changes: 20 additions & 0 deletions types/geofilter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package types

import (
"encoding/binary"
"math/rand"
"strings"
"testing"

Expand Down Expand Up @@ -408,3 +409,22 @@ func TestMatchesFilterNearPoint(t *testing.T) {
})
require.True(t, qd.MatchesFilter(poly))
}

func BenchmarkMatchesFilterContainsPoint(b *testing.B) {
us, _ := loadPolygon("testdata/us.json")
b.ResetTimer()

for i := 0; i < b.N; i++ {
p := geom.NewPoint(geom.XY).MustSetCoords(geom.Coord{
(rand.Float64() * 360) - 180,
(rand.Float64() * 180) - 90})

d, _ := wkb.Marshal(p, binary.LittleEndian)
src := ValueForType(GeoID)
src.Value = []byte(d)
gd, _ := Convert(src, StringID)
gb := gd.Value.(string)
_, qd, _ := queryTokens(QueryTypeContains, gb, 0.0)
qd.contains(us)
}
}

0 comments on commit ff69bd2

Please sign in to comment.