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.

Sign up for GitHub

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

[WIP] Basic schema enhancements for Felt basemap #37

Merged
merged 28 commits into from
Jun 1, 2023
Merged

[WIP] Basic schema enhancements for Felt basemap #37

Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
0920a06
add disputed boundaries handling
nvkelso May 25, 2023
3ea5211
add exotic water tags and min pix size
nvkelso May 25, 2023
17f8ddd
add provinces, add fallback populations by place type, add population…
nvkelso May 26, 2023
e4c53c6
add leisure, add aerodrome, add pmap:kind coallese
nvkelso May 26, 2023
a44c44d
add ref_length, split other into path and other, add pmap:link, add b…
nvkelso May 26, 2023
0ed7b0c
add pier lines
nvkelso May 26, 2023
c643234
bug fixes
nvkelso May 26, 2023
1077301
move water farther down, add pier lines, add poi labels
nvkelso May 26, 2023
53c8ac7
add calcualted network, clean ref (first network only)
nvkelso May 26, 2023
6825103
piers are man_made (not highway)
nvkelso May 26, 2023
14d0824
just under industrial
nvkelso May 26, 2023
3b55d6a
draw pier poly above water
nvkelso May 26, 2023
d8258ee
fix pier bug, city sort on min_zoom and fix anchor bug
nvkelso May 26, 2023
43eb247
register Natural Earth with Places layer
nvkelso May 26, 2023
53ce164
refactor disputed (for bug)
nvkelso May 26, 2023
74d4578
add Natural Earth places at low zooms only
nvkelso May 26, 2023
347f121
allow polygon centroid POI labels
nvkelso May 26, 2023
0079233
strip whitespace from ref, and only set ref by zoom and kind
nvkelso May 26, 2023
e675da6
add NeNames processing class
nvkelso May 26, 2023
1e2b1c5
even if an other road type, still export ref at higher zooms
nvkelso May 26, 2023
71c827a
even if an other road type, still export ref at higher zooms
nvkelso May 26, 2023
4db8422
strip after network calc
nvkelso May 26, 2023
b22c1ea
typo
nvkelso May 26, 2023
8252e77
improve pier widths
nvkelso May 26, 2023
aeef7ac
formatting
bdon May 29, 2023
f1858ae
add linting note, formatting
nvkelso May 31, 2023
5b62ac0
int and break, per PR comments
nvkelso May 31, 2023
5401363
remove very small featues to declutter the map
nvkelso May 31, 2023
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
62 changes: 61 additions & 1 deletion base/src/base_layers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,19 @@ export function nolabels_layers(source: string, c: Theme): any[] {
"fill-color": c.water,
},
},
{
bdon marked this conversation as resolved.
Show resolved Hide resolved
id: "landuse_pier",
type: "fill",
source: source,
"source-layer": "landuse",
filter: [
"any",
["==", "man_made", "pier"],
],
paint: {
"fill-color": c.earth,
},
},
{
id: "roads_tunnels_other_casing",
type: "line",
Expand Down Expand Up @@ -534,6 +547,30 @@ export function nolabels_layers(source: string, c: Theme): any[] {
visibility: casingVisibility,
},
},
{
id: "transit_pier",
type: "line",
source: source,
"source-layer": "transit",
filter: [
"any",
["==", "pmap:kind", "pier"],
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

],
paint: {
"line-color": c.minor,
"line-width": [
"interpolate",
["exponential", 1.6],
["zoom"],
12,
0,
12.5,
0.5,
20,
16,
],
},
},
{
id: "roads_minor",
type: "line",
Expand Down Expand Up @@ -1142,6 +1179,23 @@ export function labels_layers(source: string, c: Theme): any[] {
"text-halo-width": 1.5,
},
},
{
id: "pois",
bdon marked this conversation as resolved.
Show resolved Hide resolved
type: "symbol",
source: source,
minzoom: 17,
"source-layer": "pois",
layout: {
"text-font": ["NotoSans-Regular"],
"text-field": ["get", "name"],
"text-size": 12,
},
paint: {
"text-color": c.subplace_label,
"text-halo-color": c.subplace_label_halo,
"text-halo-width": 1.5,
},
},
{
id: "places_subplace",
type: "symbol",
Expand Down Expand Up @@ -1187,10 +1241,16 @@ export function labels_layers(source: string, c: Theme): any[] {
"source-layer": "places",
filter: ["==", "pmap:kind", "city"],
layout: {
"symbol-sort-key": ["number", ["get", "pmap:min_zoom"]],
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This allows San Francisco to win over Oakland (which otherwise has larger population):

You can see the Oakland dot (symbolized in a different layer) but the San Francisco text wins.

image

"text-field": "{name}",
"text-font": ["NotoSans-Bold"],
"text-size": ["step", ["get", "pmap:rank"], 0, 1, 12, 2, 10],
"text-variable-anchor": ["bottom-left"],
"text-anchor": {
stops: [
[7, "left"],
[8, "center"],
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise text was still align left align on zoom 8+ when the townspot goes away:

image

],
},
"text-radial-offset": 0.2,
},
paint: {
Expand Down
1 change: 1 addition & 0 deletions tiles/src/main/java/com/protomaps/basemap/Basemap.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ public Basemap(Boolean useTilezen) {
var place = new Places();
registerHandler(place);
registerSourceHandler("osm", place);
registerSourceHandler("ne", place::processNe);

var poi = new Pois();
registerHandler(poi);
Expand Down
13 changes: 10 additions & 3 deletions tiles/src/main/java/com/protomaps/basemap/layers/Boundaries.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ public void processFeature(SourceFeature sf, FeatureCollector features) {
List<OsmReader.RelationMember<AdminRecord>> recs = sf.relationInfo(AdminRecord.class);
if (recs.size() > 0) {
OptionalInt minAdminLevel = recs.stream().mapToInt(r -> r.relation().adminLevel).min();
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NOTE: This looks like it prevents the City and County of San Francisco from appearing as a locality boundary line, instead it only appears as a county boundary line. An odd case, but this is true for several other "global cities" in the USA and elsewhere, too.

OptionalInt disputed = recs.stream().mapToInt(r -> r.relation().disputed).max();
var line =
features.line(this.name()).setId(FeatureId.create(sf)).setMinPixelSize(0).setAttr("pmap:min_admin_level",
minAdminLevel.getAsInt());
Expand All @@ -40,17 +41,23 @@ public void processFeature(SourceFeature sf, FeatureCollector features) {
} else {
line.setMinZoom(10);
}

if( disputed.getAsInt() == 1 ) {
line.setAttr("disputed", 1);
}
}
}
}

@Override
public List<OsmRelationInfo> preprocessOsmRelation(OsmElement.Relation relation) {
if (relation.hasTag("type", "boundary") && relation.hasTag("boundary", "administrative")) {
if (relation.hasTag("type", "boundary") && (relation.hasTag("boundary", "administrative") || relation.hasTag("boundary", "disputed"))) {
Integer adminLevel = Parse.parseIntOrNull(relation.getString("admin_level"));
Integer disputed = relation.hasTag("boundary", "disputed") ? 1 : 0;
Copy link
Collaborator Author

@nvkelso nvkelso May 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: This seems to always eval to 0 in error

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nvkelso what relations/area are you testing on? on observation with Overpass Turbo it seems many relations with boundary=disputed lack the type=boundary tag, eg https://www.openstreetmap.org/relation/13562322

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Line of Control in Cyprus (r/13562322)is an odd ball – it's admin_level is 3 for starters.

I was testing around Israel as the build there has a smaller bounding box than India or China and more admin_level = 2 features.

  • Israel - Egypt boundary way with relevant relations: w/95211614
  • Israel - Gaza boundary way with relevant relations: w/25057102

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to evaluate to 1 on this relation: https://www.openstreetmap.org/relation/1703814


if (adminLevel == null || adminLevel > 8)
return null;
return List.of(new AdminRecord(relation.id(), adminLevel));
return List.of(new AdminRecord(relation.id(), adminLevel, disputed));
}
return null;
}
Expand All @@ -64,5 +71,5 @@ public List<VectorTile.Feature> postProcess(int zoom, List<VectorTile.Feature> i
);
}

private record AdminRecord(long id, int adminLevel) implements OsmRelationInfo {}
private record AdminRecord(long id, int adminLevel, int disputed) implements OsmRelationInfo {}
}
8 changes: 8 additions & 0 deletions tiles/src/main/java/com/protomaps/basemap/layers/PhysicalLine.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,16 @@ public void processFeature(SourceFeature sf, FeatureCollector features) {
.setId(FeatureId.create(sf))
.setAttr("waterway", sf.getString("waterway"))
.setAttr("natural", sf.getString("natural"))
// Add less common attributes only at higher zooms
.setAttrWithMinzoom("bridge", sf.getString("bridge"), 12)
.setAttrWithMinzoom("tunnel", sf.getString("tunnel"), 12)
.setAttrWithMinzoom("layer", sf.getString("layer"), 12)
.setZoomRange(12, 15);

if( sf.hasTag("intermittent", "yes" ) ) {
feat.setAttr("intermittent", 1);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For way 316401077 in israel-and-palestine GeoFabrik:

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bdon up to you if you want this to be a boolean in the MVT or an int

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tilezen uses a boolean so let's converge on that later

}

String kind = "other";
if (sf.hasTag("waterway")) {
kind = "waterway";
Expand Down
128 changes: 109 additions & 19 deletions tiles/src/main/java/com/protomaps/basemap/layers/Places.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import com.onthegomap.planetiler.util.Parse;
import com.protomaps.basemap.feature.FeatureId;
import com.protomaps.basemap.names.OsmNames;
import com.protomaps.basemap.names.NeNames;
import java.util.ArrayList;
import java.util.List;

Expand All @@ -17,10 +18,64 @@ public String name() {
return "places";
}

public void processNe(SourceFeature sf, FeatureCollector features) {
var sourceLayer = sf.getSourceLayer();
var kind = "";
var kind_detail = "";

var theme_min_zoom = 0;
var theme_max_zoom = 0;
if (sourceLayer.equals("ne_10m_populated_places")) {
theme_min_zoom = 1;
theme_max_zoom = 8;
}

// Test for props because of Natural Earth funk
if( sf.isPoint() && sf.hasTag("featurecla") && sf.hasTag("min_zoom" ) ) {
switch (sf.getString("featurecla")) {
case "Admin-0 capital":
case "Admin-0 capital alt":
case "Admin-0 region capital":
kind = "city";
break;
case "Admin-1 capital":
case "Admin-1 region capital":
kind = "city";
break;
case "Populated place":
kind = "city";
break;
case "Historic place":
kind = "locality";
kind_detail = "hamlet";
break;
case "Scientific station":
kind = "locality";
kind_detail = "scientific_station";
break;
}
}

if( kind != "" ) {
var feat = features.point(this.name())
.setAttr("name", sf.getString("name"))
.setAttr("pmap:min_zoom", sf.getLong("min_zoom"))
.setZoomRange(sf.getString("min_zoom") == null ? theme_min_zoom : (int)Double.parseDouble(sf.getString("min_zoom")), theme_max_zoom)
.setAttr("pmap:kind", kind)
.setAttr("pmap:kind_detail", kind_detail)
.setAttr("population", sf.getString("pop_max"))
.setAttr("population_rank", sf.getString("rank_max"))
.setAttr("wikidata_id", sf.getString("wikidata"))
.setBufferPixels(128);

NeNames.setNeNames(feat, sf, 0);
}
}
@Override
public void processFeature(SourceFeature sf, FeatureCollector features) {
if (sf.isPoint() &&
(sf.hasTag("place", "suburb", "town", "village", "neighbourhood", "city", "country", "state"))) {
(sf.hasTag("place", "suburb", "town", "village", "neighbourhood", "city", "country", "state", "province"))) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were missing province

Integer population = sf.getString("population") == null ? 0 : (int)Double.parseDouble(sf.getString("population"));
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We always want to have population values, default to 0 if missing and backfill by place type down below.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think plain int is fine then if it's never null

var feat = features.point(this.name())
.setId(FeatureId.create(sf))
.setAttr("place", sf.getString("place"))
Expand All @@ -31,38 +86,73 @@ public void processFeature(SourceFeature sf, FeatureCollector features) {

if (sf.hasTag("place", "country")) {
feat.setAttr("pmap:kind", "country")
.setZoomRange(0, 15);
.setZoomRange(0, 9);
bdon marked this conversation as resolved.
Show resolved Hide resolved
} else if (sf.hasTag("place", "state", "province")) {
feat.setAttr("pmap:kind", "state")
.setZoomRange(4, 15);
.setZoomRange(4, 11);
bdon marked this conversation as resolved.
Show resolved Hide resolved
} else if (sf.hasTag("place", "city")) {
feat.setAttr("pmap:kind", "city")
.setZoomRange(4, 15);

if (sf.getString("population") != null) {
Integer population = Parse.parseIntOrNull(sf.getString("population"));
if (population != null) {
feat.setAttr("population", population);
feat.setSortKey((int) Math.log(population));
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This moved farther down...

// TODO: use label grid
} else {
feat.setSortKey(0);
}
}

} else if (sf.hasTag("place", "suburb")) {
feat.setAttr("pmap:kind", "neighbourhood")
.setZoomRange(8, 15);
if ( population.equals(0) ) {
population = 10000;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's where we start backfilling population by place type.

}
} else if (sf.hasTag("place", "town")) {
feat.setAttr("pmap:kind", "neighbourhood")
.setZoomRange(8, 15);
if ( population.equals(0) ) {
population = 5000;
}
} else if (sf.hasTag("place", "village")) {
feat.setAttr("pmap:kind", "neighbourhood")
.setZoomRange(10, 15);
.setZoomRange(10, 15);
if ( population.equals(0) ) {
population = 2000;
}
} else if (sf.hasTag("place", "suburb")) {
feat.setAttr("pmap:kind", "neighbourhood")
.setZoomRange(8, 15);
} else {
feat.setAttr("pmap:kind", "neighbourhood")
.setZoomRange(12, 15);
}

if (population != null) {
feat.setAttr("population", population);
feat.setSortKey((int) Math.log(population));
// TODO: use label grid
} else {
feat.setSortKey(0);
}

Integer population_rank = 0;

int[] pop_breaks = {
1000000000,
100000000,
50000000,
20000000,
10000000,
5000000,
1000000,
500000,
200000,
100000,
50000,
20000,
10000,
5000,
2000,
1000,
200,
0};

for (int i = 0; i < pop_breaks.length; i++) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't working in Java, but works fine in Python. Help!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you want to break once you find a result in the array that matches, right?

it seems like population_rank at the top is assigned to 0 or the OSM value. Since there's not really a case where it's nullable I don't think you need to use the Integer type, just plain int should be OK.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOL, yes. Fixed via 5b62ac0

image image

if( population >= pop_breaks[i]) {
population_rank = pop_breaks.length - i;
}
}

feat.setAttr("population_rank", population_rank);
}
}

Expand Down
Loading