diff --git a/geom/multipolygon.go b/geom/multipolygon.go index 3fceac8..073ad5c 100644 --- a/geom/multipolygon.go +++ b/geom/multipolygon.go @@ -141,14 +141,17 @@ func BuildRelGeometry(rel *element.Relation, rings []*Ring, srid int) (*geos.Geo // add ring as hole or shell if ringIsHole(rings, j) { rings[i].holes[rings[j]] = true + rings[i].outer = false } else { shells[rings[j]] = true + rings[i].outer = true } } } if rings[i].containedBy == -1 { // add as shell if it is not a hole shells[rings[i]] = true + rings[i].outer = true } g.PreparedDestroy(testGeom) } @@ -157,7 +160,6 @@ func BuildRelGeometry(rel *element.Relation, rings []*Ring, srid int) (*geos.Geo for shell, _ := range shells { var interiors []*geos.Geom for hole, _ := range shell.holes { - hole.MarkInserted(rel.Tags) ring := g.Clone(g.ExteriorRing(hole.geom)) g.Destroy(hole.geom) if ring == nil { @@ -165,7 +167,6 @@ func BuildRelGeometry(rel *element.Relation, rings []*Ring, srid int) (*geos.Geo } interiors = append(interiors, ring) } - shell.MarkInserted(rel.Tags) exterior := g.Clone(g.ExteriorRing(shell.geom)) g.Destroy(shell.geom) if exterior == nil { @@ -198,10 +199,20 @@ func BuildRelGeometry(rel *element.Relation, rings []*Ring, srid int) (*geos.Geo g.DestroyLater(result) - insertedWays := make(map[int64]bool) - for _, r := range rings { - for id, _ := range r.inserted { - insertedWays[id] = true + outer := make(map[int64]struct{}) + for i := range rings { + if rings[i].outer { + for _, w := range rings[i].ways { + outer[w.Id] = struct{}{} + } + } + } + for i := range rel.Members { + mid := rel.Members[i].Id + if _, ok := outer[mid]; ok { + rel.Members[i].Role = "outer" + } else { + rel.Members[i].Role = "inner" } } diff --git a/geom/ring.go b/geom/ring.go index 2de5048..022ec64 100644 --- a/geom/ring.go +++ b/geom/ring.go @@ -13,6 +13,7 @@ type Ring struct { holes map[*Ring]bool containedBy int area float64 + outer bool inserted map[int64]bool } @@ -24,40 +25,6 @@ func (r *Ring) TryClose(maxRingGap float64) bool { return element.TryCloseWay(r.refs, r.nodes, maxRingGap) } -func (r *Ring) MarkInserted(tags element.Tags) { - if r.inserted == nil { - r.inserted = make(map[int64]bool) - } - for _, w := range r.ways { - if tagsSameOrEmpty(tags, w.Tags) { - r.inserted[w.Id] = true - } - } -} - -func tagsSameOrEmpty(a, b element.Tags) bool { - if len(b) == 0 { - return true - } - for k, v := range a { - if k == "name" { - continue - } - if cmp, ok := b[k]; !ok || v != cmp { - return false - } - } - for k, v := range b { - if k == "name" { - continue - } - if cmp, ok := a[k]; !ok || v != cmp { - return false - } - } - return true -} - func NewRing(way *element.Way) *Ring { ring := Ring{} ring.ways = []*element.Way{way} diff --git a/geom/ring_test.go b/geom/ring_test.go index 347c3aa..8b8918b 100644 --- a/geom/ring_test.go +++ b/geom/ring_test.go @@ -1,9 +1,10 @@ package geom import ( - "github.com/omniscale/imposm3/element" "sort" "testing" + + "github.com/omniscale/imposm3/element" ) func TestRingMerge(t *testing.T) { @@ -210,42 +211,3 @@ func TestRingMergePermutations(t *testing.T) { } } } - -func TestTagsSameOrEmpty(t *testing.T) { - var a, b element.Tags - - a = element.Tags{"natural": "water"} - b = element.Tags{"natural": "water"} - if tagsSameOrEmpty(a, b) != true { - t.Fatal(a, b) - } - - a = element.Tags{"natural": "water"} - b = element.Tags{"natural": "water", "name": "bar"} - if tagsSameOrEmpty(a, b) != true { - t.Fatal(a, b) - } - if tagsSameOrEmpty(b, a) != true { - t.Fatal(a, b) - } - - a = element.Tags{"natural": "water"} - b = element.Tags{} - if tagsSameOrEmpty(a, b) != true { - t.Fatal(a, b) - } - - if tagsSameOrEmpty(b, a) != false { - t.Fatal(a, b) - } - - a = element.Tags{"natural": "water"} - b = element.Tags{"landusage": "forest", "natural": "water"} - if tagsSameOrEmpty(a, b) != false { - t.Fatal(a, b) - } - if tagsSameOrEmpty(b, a) != false { - t.Fatal(a, b) - } - -} diff --git a/mapping/matcher.go b/mapping/matcher.go index 51911c6..ab94559 100644 --- a/mapping/matcher.go +++ b/mapping/matcher.go @@ -1,8 +1,6 @@ package mapping -import ( - "github.com/omniscale/imposm3/element" -) +import "github.com/omniscale/imposm3/element" func (m *Mapping) PointMatcher() NodeMatcher { mappings := make(TagTables) @@ -124,7 +122,9 @@ func (tm *tagMatcher) match(tags *element.Tags) []Match { } // SelectRelationPolygons returns a slice of all members that are already -// imported with a relation with tags. +// imported as part of the relation. +// Outer members are "imported" if they share the same destination table. Inner members +// are "imported" when they also share the same key/value. func SelectRelationPolygons(polygonTagMatcher RelWayMatcher, rel *element.Relation) []element.Member { relMatches := polygonTagMatcher.MatchRelation(rel) result := []element.Member{} @@ -133,13 +133,16 @@ func SelectRelationPolygons(polygonTagMatcher RelWayMatcher, rel *element.Relati continue } memberMatches := polygonTagMatcher.MatchWay(m.Way) - if matchEquals(relMatches, memberMatches) { + if m.Role == "outer" && dstEquals(relMatches, memberMatches) { + result = append(result, m) + } else if matchEquals(relMatches, memberMatches) { result = append(result, m) } } return result } +// matchEquals returns true if both matches share key/value and table func matchEquals(matchesA, matchesB []Match) bool { for _, matchA := range matchesA { for _, matchB := range matchesB { @@ -152,3 +155,15 @@ func matchEquals(matchesA, matchesB []Match) bool { } return false } + +// dstEquals returns true if both matches share a single destination table +func dstEquals(matchesA, matchesB []Match) bool { + for _, matchA := range matchesA { + for _, matchB := range matchesB { + if matchA.Table == matchB.Table { + return true + } + } + } + return false +} diff --git a/mapping/matcher_test.go b/mapping/matcher_test.go index cfb3092..6600edd 100644 --- a/mapping/matcher_test.go +++ b/mapping/matcher_test.go @@ -26,8 +26,15 @@ func makeMember(id int64, tags element.Tags) element.Member { element.OSMElem{id, tags, nil}, []int64{0, 1, 2, 0}, // fake closed way, req. for SelectRelationPolygons nil} - return element.Member{Id: id, Type: element.WAY, Role: "outer", Way: way} + return element.Member{Id: id, Type: element.WAY, Role: "", Way: way} +} +func makeMemberRole(id int64, tags element.Tags, role string) element.Member { + way := &element.Way{ + element.OSMElem{id, tags, nil}, + []int64{0, 1, 2, 0}, // fake closed way, req. for SelectRelationPolygons + nil} + return element.Member{Id: id, Type: element.WAY, Role: role, Way: way} } func TestSelectRelationPolygonsSimple(t *testing.T) { @@ -125,3 +132,28 @@ func TestSelectRelationPolygonsMultipleTags(t *testing.T) { t.Fatal(filtered) } } + +func TestSelectRelationPolygonsMultipleTagsOnWay(t *testing.T) { + mapping, err := NewMapping("test_mapping.json") + if err != nil { + t.Fatal(err) + } + r := element.Relation{} + r.Tags = element.Tags{"waterway": "riverbank"} + r.Members = []element.Member{ + makeMemberRole(0, element.Tags{"waterway": "riverbank", "natural": "water"}, "outer"), + makeMemberRole(1, element.Tags{"natural": "water"}, "inner"), + makeMemberRole(2, element.Tags{"place": "islet"}, "inner"), + } + filtered := SelectRelationPolygons( + mapping.PolygonMatcher(), + &r, + ) + + if len(filtered) != 1 { + t.Fatal(filtered) + } + if filtered[0].Id != 0 { + t.Fatal(filtered) + } +} diff --git a/mapping/test_mapping.json b/mapping/test_mapping.json index 48ce8f3..5b14963 100644 --- a/mapping/test_mapping.json +++ b/mapping/test_mapping.json @@ -158,6 +158,7 @@ ], "natural": [ "wood", + "water", "land", "scrub", "wetland", @@ -201,6 +202,9 @@ "highway": [ "pedestrian", "footway" + ], + "waterway": [ + "riverbank" ] } }, @@ -857,54 +861,6 @@ "__any__" ] } - }, - "waterareas": { - "fields": [ - { - "type": "id", - "name": "osm_id", - "key": null - }, - { - "type": "geometry", - "name": "geometry", - "key": null - }, - { - "type": "string", - "name": "name", - "key": "name" - }, - { - "type": "mapping_value", - "name": "type", - "key": null - }, - { - "type": "pseudoarea", - "name": "area", - "key": null - } - ], - "type": "polygon", - "mapping": { - "waterway": [ - "riverbank" - ], - "landuse": [ - "basin", - "reservoir" - ], - "natural": [ - "water" - ], - "amenity": [ - "swimming_pool" - ], - "leisure": [ - "swimming_pool" - ] - } } } } diff --git a/test/complete_db.osm b/test/complete_db.osm index 4ebfd11..ce3c6eb 100644 --- a/test/complete_db.osm +++ b/test/complete_db.osm @@ -785,6 +785,79 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/complete_db_test.py b/test/complete_db_test.py index be60f9e..0129982 100644 --- a/test/complete_db_test.py +++ b/test/complete_db_test.py @@ -124,6 +124,27 @@ def test_node_way_inserted_twice(): assert rows[0]['type'] == 'residential' assert rows[1]['type'] == 'tram' +def test_outer_way_not_inserted(): + """Outer way with different tag is not inserted twice into same table""" + farm = t.query_row(t.db_conf, 'osm_landusages', -19001) + assert farm['type'] == 'farmland' + assert not t.query_row(t.db_conf, 'osm_landusages', 19001) + + farmyard = t.query_row(t.db_conf, 'osm_landusages', 19002) + assert farmyard['type'] == 'farmyard' + +def test_outer_way_inserted(): + """Outer way with different tag is inserted twice into different table""" + farm = t.query_row(t.db_conf, 'osm_landusages', 19101) + assert farm['type'] == 'farm' + assert not t.query_row(t.db_conf, 'osm_landusages', -19101) + + farmyard = t.query_row(t.db_conf, 'osm_landusages', 19102) + assert farmyard['type'] == 'farmyard' + + admin = t.query_row(t.db_conf, 'osm_admin', -19101) + assert admin['type'] == 'administrative' + def test_node_way_ref_after_delete_1(): """Nodes refereces way""" data = t.cache_query(nodes=[20001, 20002], deps=True) @@ -159,6 +180,7 @@ def test_relation_ways_inserted(): park = t.query_row(t.db_conf, 'osm_landusages', -9201) assert park['type'] == 'park' assert park['name'] == '9209' + assert not t.query_row(t.db_conf, 'osm_landusages', 9201) # outer ways of multipolygon stand for their own road = t.query_row(t.db_conf, 'osm_roads', 9209)