Skip to content

Commit

Permalink
Invert s2 loop instead of rebuilding (hypermodeinc#4782)
Browse files Browse the repository at this point in the history
* Added test for hypermodeinc#4782

To show that inverting the loop makes a difference.

* Invert s2 loop instead of rebuilding

I was using loopFromPolygon in my own project and I noticed a weird
issue with some polygons which results in the contains query always
returning true, using Invert instead of rebuilding the loop here seems
to fix it.
  • Loading branch information
sams96 authored Mar 16, 2020
1 parent b7e64f1 commit 0832fc1
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 1 deletion.
10 changes: 10 additions & 0 deletions types/geofilter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,16 @@ func TestMatchesFilterContainsPoint(t *testing.T) {
_, qd, err = queryTokens(QueryTypeContains, data, 0.0)
require.NoError(t, err)
require.False(t, qd.MatchesFilter(us))

// Test with a different polygon (Sudan)
sudan, err := loadPolygon("testdata/sudan.json")
require.NoError(t, err)

p = geom.NewPoint(geom.XY).MustSetCoords(geom.Coord{0, 0})
data = formDataPoint(t, p)
_, qd, err = queryTokens(QueryTypeContains, data, 0.0)
require.NoError(t, err)
require.False(t, qd.MatchesFilter(sudan))
}

/*
Expand Down
2 changes: 1 addition & 1 deletion types/s2index.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func loopFromPolygon(p *geom.Polygon) (*s2.Loop, error) {

// Since our clockwise check was approximate, we check the cap and reverse if needed.
if l.CapBound().Radius().Degrees() > 90 {
l = loopFromRing(r, !reverse)
l.Invert()
}
return l, nil
}
Expand Down
1 change: 1 addition & 0 deletions types/testdata/sudan.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"type":"Feature","properties":{"scalerank":1,"featurecla":"Admin-0 country","LABELRANK":3,"SOVEREIGNT":"Sudan","SOV_A3":"SDN","ADM0_DIF":0,"LEVEL":2,"TYPE":"Sovereign country","ADMIN":"Sudan","ADM0_A3":"SDN","GEOU_DIF":0,"GEOUNIT":"Sudan","GU_A3":"SDN","SU_DIF":0,"SUBUNIT":"Sudan","SU_A3":"SDN","BRK_DIFF":0,"NAME":"Sudan","NAME_LONG":"Sudan","BRK_A3":"SDN","BRK_NAME":"Sudan","BRK_GROUP":null,"ABBREV":"Sudan","POSTAL":"SD","FORMAL_EN":"Republic of the Sudan","FORMAL_FR":null,"NAME_CIAWF":"Sudan","NOTE_ADM0":null,"NOTE_BRK":null,"NAME_SORT":"Sudan","NAME_ALT":null,"MAPCOLOR7":2,"MAPCOLOR8":6,"MAPCOLOR9":4,"MAPCOLOR13":1,"POP_EST":37345935,"POP_RANK":15,"GDP_MD_EST":176300,"POP_YEAR":2017,"LASTCENSUS":2008,"GDP_YEAR":2016,"ECONOMY":"6. Developing region","INCOME_GRP":"4. Lower middle income","WIKIPEDIA":-99,"FIPS_10_":"SU","ISO_A2":"SD","ISO_A3":"SDN","ISO_A3_EH":"SDN","ISO_N3":"729","UN_A3":"729","WB_A2":"SD","WB_A3":"SDN","WOE_ID":-90,"WOE_ID_EH":23424952,"WOE_NOTE":"Almost all FLickr photos are in the north.","ADM0_A3_IS":"SDN","ADM0_A3_US":"SDN","ADM0_A3_UN":-99,"ADM0_A3_WB":-99,"CONTINENT":"Africa","REGION_UN":"Africa","SUBREGION":"Northern Africa","REGION_WB":"Sub-Saharan Africa","NAME_LEN":5,"LONG_LEN":5,"ABBREV_LEN":5,"TINY":-99,"HOMEPART":1,"MIN_ZOOM":0,"MIN_LABEL":3,"MAX_LABEL":8},"bbox":[21.93681,8.229188,38.41009,22],"geometry":{"type":"Polygon","coordinates":[[[24.567369,8.229188],[23.805813,8.666319],[23.459013,8.954286],[23.394779,9.265068],[23.55725,9.681218],[23.554304,10.089255],[22.977544,10.714463],[22.864165,11.142395],[22.87622,11.38461],[22.50869,11.67936],[22.49762,12.26024],[22.28801,12.64605],[21.93681,12.58818],[22.03759,12.95546],[22.29658,13.37232],[22.18329,13.78648],[22.51202,14.09318],[22.30351,14.32682],[22.56795,14.94429],[23.02459,15.68072],[23.88689,15.61084],[23.83766,19.58047],[23.85,20],[25,20.00304],[25,22],[29.02,22],[32.9,22],[36.86623,22],[37.18872,21.01885],[36.96941,20.83744],[37.1147,19.80796],[37.48179,18.61409],[37.86276,18.36786],[38.41009,17.998307],[37.904,17.42754],[37.16747,17.26314],[36.85253,16.95655],[36.75389,16.29186],[36.32322,14.82249],[36.42951,14.42211],[36.27022,13.56333],[35.86363,12.57828],[35.26049,12.08286],[34.83163,11.31896],[34.73115,10.91017],[34.25745,10.63009],[33.96162,9.58358],[33.97498,8.68456],[33.963393,9.464285],[33.824963,9.484061],[33.842131,9.981915],[33.721959,10.325262],[33.206938,10.720112],[33.086766,11.441141],[33.206938,12.179338],[32.743419,12.248008],[32.67475,12.024832],[32.073892,11.97333],[32.314235,11.681484],[32.400072,11.080626],[31.850716,10.531271],[31.352862,9.810241],[30.837841,9.707237],[29.996639,10.290927],[29.618957,10.084919],[29.515953,9.793074],[29.000932,9.604232],[28.966597,9.398224],[27.97089,9.398224],[27.833551,9.604232],[27.112521,9.638567],[26.752006,9.466893],[26.477328,9.55273],[25.962307,10.136421],[25.790633,10.411099],[25.069604,10.27376],[24.794926,9.810241],[24.537415,8.917538],[24.194068,8.728696],[23.88698,8.61973],[24.567369,8.229188]]]}}

0 comments on commit 0832fc1

Please sign in to comment.