While reviewing #1255, it became apparent that the ETL documentation for the landcover layer was incomplete. This PR updates the ETL documentation in `generalized.sql` so that each table has a node in the ETL diagram.
Avoid redirection of error messages to output files
- build/openmaptiles.tm2source/data.yml
- build/mapping.yaml
Currently, if an error occurs when `make all` creates any of these files, the error message is not printed to the console.
```
docker-compose run --rm --user=1000:1000 openmaptiles-tools generate-tm2source openmaptiles.yaml --host="postgres" --port=5432 --database="openmaptiles" --user="openmaptiles" --password="openmaptiles" > build/openmaptiles.tm2source/data.yml
Creating openmaptiles_openmaptiles-tools_run ... done
ERROR: 1
make: *** [Makefile:257: build/openmaptiles.tm2source/data.yml] Error 1
```
An inexperienced user would not know where to look for the details.
With this PR, the error is sent to the console:
```
docker-compose run --rm --user=1000:1000 openmaptiles-tools bash -c \
'generate-tm2source openmaptiles.yaml --host="postgres" --port=5432 --database="openmaptiles" --user="openmaptiles" --password="openmaptiles" > build/openmaptiles.tm2source/data.yml'
Creating openmaptiles_openmaptiles-tools_run ... done
Could not parse /tileset/layers/aeroway/mapping.yaml
found undefined alias 'refkjbkjkhh'
in "/tileset/layers/aeroway/mapping.yaml", line 94, column 7
ERROR: 1
make: *** [Makefile:257: build/openmaptiles.tm2source/data.yml] Error 1
```
Redirection of stdout is now done in the container, leaving stderr to go to the host.
This test applies 2-3 months of weekly updates on the `europe/monaco` extract from [Geofabrik](http://download.geofabrik.de/europe/).
It is worth noting that the contents of the updates may vary, and some SQL update flows may not be tested if the relevant changes did not occur during that period.
Resolves#1226
This PR fixes a bug in the park polygon update code introduced in #1160. Because park polygons at zoom 4 are dissolved, the centroid is not present in the table at that layer. Thus, a separate update trigger is needed to avoid errors. This PR adds the separate trigger for the zoom 4 park layer.
This PR likely resolves an unidentified blocker for #1245
Fixes#1156Fixes#810Fixes#1228
This PR replaces `osmborder`, which is no longer maintained, with `imposm` mappings and SQL code to generate borders. Key features that were moved into the imposm/SQL layer:
1. Grouping by `osm_id` and aggregating by lowest `admin_level` value so that there's only one copy of ways that are members of multiple relations.
2. Filtering out of point features in boundary relations (typically `admin_centre` and `label` roles).
3. Move disputed boundary detection logic into SQL.
This will increase the database size slightly because of the limits of what imposm can do, as some of the filtering is done in the SQL layer after importing, rather than being done in `osmborder`.
This PR is a bugfix for the `transportation_name` layer. Currently, the updates process does not work, as noted by @zstadler in #1230. The issue is that the computed `concurrency_index` column in `osm_route_member` is never updated after the initial updates. Therefore, whenever route member ways are updated, they will lose their route concurrency index, and hence route concurrencies will be lost in the `transportation_name` tiles. This PR adds the missing concurrency index update code to the transportation/network update triggers.
This PR as written also incorporates the fixes in #1230 and #1233.
The following SQL is a unit test to demonstrate that these triggers work correctly:
```
select count(*) from osm_route_member where concurrency_index > 5;
select count(*) from osm_transportation_name_network where route_6 is not null;
update osm_route_member set concurrency_index=NULL where concurrency_index > 5;
select count(*) from osm_route_member where concurrency_index > 5;
select count(*) from osm_transportation_name_network where route_6 is not null;
```
If working correctly, both pairs of `count(*)` values should return the same pair of numbers.
Because of the parallel processing of `import-sql` the error message is printed in the middle of the output. With this PR the error is displayed again at the end:
Issue: https://github.com/openmaptiles/openmaptiles-tools/pull/370
```
awk '1{print; fflush()} $$0~".*ERROR" {txt=$$0} END{ if(txt){print "\n*** ERROR detected, aborting:"; print txt; exit(1)} }'
```
Explanation:
- `1{print; fflush()}` means if true then print output -> `1{}` same as `if(true){}`
- `$$0~".*ERROR" {txt=$$0}` get first argument `$$0` (line of output) and check if it contains (regex string) `ERROR`. If true save line to var `txt`: `{txt=$$0}`
- `END{ ... }` if last line of output is reached
- `if(txt){print "\n*** ERROR detected, aborting:"; print txt; exit(1)}` if error was found in a line / var `txt` is existing print it out
While troubleshooting #1230, I discovered that there were cases where a way was a member of the same route relation more than once, such as: https://www.openstreetmap.org/way/17439235
<img src="https://user-images.githubusercontent.com/3254090/133726616-acd01332-343e-4930-ad3d-1fb3c3190a4c.png" width=280/>
This segment would end up showing duplicate highway shields in a rendered style, which is not desirable. While I personally think this style of tagging is wrong, it's a relatively easy fix to remove those duplicates. This PR replaces `ROW_NUMBER()` with `DENSE_RANK()` in the concurrency index generation code and adds a `DISTINCT` constraint on the concurrency join to handle multiple member rows with the same concurrency index.
Since I was in this file, I also fixed the copy/paste error in generating the route names.
Screenshot from database showing correct mapping:
The tag `landuse=park` is a rare tagging mistake of `leisure=park`. The OSM wiki [lists this tag as a mistake](https://wiki.openstreetmap.org/wiki/Tag:leisure%3Dpark). It is included in the landcover layer with a note that there were (at the time) 600 parks tagged in this way. There are currently a bit over 100 objects tagged with this mistake. Based on the wiki documentation and obvious decline of this tag, we should remove it so that we're not introducing mapping mistakes into the database and not inadvertently providing mapper feedback that encourages the use of mistake tagging.
This PR is a refactor. The purpose of this PR is to refactor the logic in the z12-z14 transportation layer to increase readability and reduce code complexity.
In addition, this fixes bugs in the original filter code:
* Create filter parity between `service` and `service_construction` highway classes.
* Removal of unintended `class` values `minor_construction`, `path_construction`, `service_construction`, and `track_construction` from zoom 12 filter by explicitly listing the highway classes that are included.
On some systems the `bc` command is not installed, so I added a check if the command can be used.
It is a simply calculation from KB to GB. If `bc` is missing, then it is displayed in KB.
fix: #1151
Adding -T fixes the issue when users are unable to stop tile genartion with ctrl+c
For some reason it doesn't work without it, even though the process of building
and running both docker images is the same.
This PR adds support for several features in the `natural` key, including `saddle`, `ridge`, `arete`, and `cliff`. These features all currently render in openstreetmap-carto, so adding these features would advance OpenMapTiles towards its goal of parity with the standard renderer, as indicated in https://github.com/openstreetmap/operations/issues/565#issuecomment-907303115.
This PR also adds the features requested in #274, with the exception of valleys, which I've left out of this PR because there are different complexities to mapping valleys that should be addressed as a separate (future) PR.
### Examples of mountain features in paper maps
These features are regularly found in American-style maps and thus is of interest to the openstreetmap-americana map style. Below are examples of these mountain-related features found in general purpose maps:
<img src="https://user-images.githubusercontent.com/3254090/131270340-af80c7bf-9416-40a3-9b64-56418abb2aef.png" width=400/>
<img src="https://user-images.githubusercontent.com/3254090/131270375-b9eb2095-7708-443d-b7bc-22bf8adab721.png" width=400/>
<img src="https://user-images.githubusercontent.com/3254090/131270423-64447ad3-d90a-4615-82e1-91175a8f8a6b.png" width=400/>
<img src="https://user-images.githubusercontent.com/3254090/131270506-2248db9f-ded5-443f-82ed-b0e8d4d12a70.png" width=400/>
### Approach
This PR extends the existing `mountain_peak` layer by adding other mountain features. We may want to consider renaming this layer in version 4.0 to be more inclusive of other mountain features including the potential future addition of valleys. However, the features added in this PR are associated with mountains and so their inclusion in this layer is the most appropriate location.
A new `osm_mountain_linestring` mapping maps the new linear mountain features, with similar ranking logic as is used for the existing `peak`/`volcano` features. Additionally, `natural=saddle` is added to the `osm_peak_point` mapping and ranked using the formula for peaks. Since saddles will have lower elevations than peaks, important saddles will be preempted by important peaks.
The new linestring features are rendered only at zoom 13-14, in order to match the zoom at which they appear in openstreetmap-carto. However, it may be appropriate in a future PR to extend the rendering of these features as generalized linestrings at lower zooms.
### Test Renderings
Below is a test rendering showing aretes and peaks in Austria, just north of the Swiss border, followed by a screen shot of the [same location](https://www.openstreetmap.org/#map=14/46.8682/10.0863) in openstreetmap-carto:
**OpenMapTiles, zoom 14**
<img src="https://user-images.githubusercontent.com/3254090/131271258-5cd90bdb-cac2-41d8-887f-b4bf6be83673.png" width=400/>
**openstreetmap-carto, zoom r14/v13:**
<img src="https://user-images.githubusercontent.com/3254090/131271332-32d5bcfc-41c6-4625-829c-df063b7af523.png" width=400/>
While importing multiple .pbf files into postgres I was confronted with some unnecessary problems:
1. I wanted to use the borders of the planet which I had already generated. But `make import-borders` needs a .pbf file which is not necessary if the line.csv file is already existing -> So I added a check if the line.csv is **not** existing it checks for the `area parameter` / .pbf file
2. `make generate-tiles-pg` is only possible with a specified `area parameter` BUT the area is not used / needed for generation -> So I removed the area check
Fixes#1191
This PR suppresses `highway=service` at zoom 12, where it is not a useful level of detail. This makes OpenMapTiles consistent with openstreetmap-carto, which does not begin showing `highway=service` until raster zoom 14 / vector zoom 13.
Additionally, this PR suppresses `highway=service` + `service=parking_aisle` / `service=driveway` from zooms 12-13, as this detail is excessive below zoom 14. As a point of comparison, openstreetmap-carto does not begin showing `service=parking_aisle` / `service=driveway` until raster zoom 16 (vector zoom 15).
Fixes#948
This PR does the following:
1. Changes the trigger for the PR comment updater from the cron method to workflow_run, triggered on completion of the test cases. This should remove the delay between the completion of the performance tests and the updating of the corresponding comment in the PR.
2. Separates the integrity check and performance check into separate workflows and allows them to run in parallel. This will allow the project to take advantage of multiple CI runners if they're available (which appears to be the case).
In addition, this fixes an issue with post-merge undeleted/updated branches on PRs. The current "cron" method causes the CI to run the pr-update job over and over forever, unnecessarily.
As described in github/docs#799, and the [github docs](https://docs.github.com/en/actions/reference/events-that-trigger-workflows#workflow_run), a `workflow_run` trigger will only fire when the workflow file is on the main branch. Thus, this change will not fire the PR updater on this PR. Thus there's no way to test this working properly without merging onto master and then testing on one of the other PRs.
Fixes#1055
This PR moves Natural Earth zoom 7.7 boundary lines into zoom 4.
> Hi, thank you for PR. In github-actions statistics there is a pretty high size increase in zooms 3, 2, 1, which are using data from zoom 4. Maybe keep your change to move `min_zoom <= 7.7` just for zoom 4, but for zoom 3 add a condition `min_zoom <= 7` (that subsequently the condition bubbles into zoom 2 and 1). What do you think?
I agree that this makes sense, and thank you for pointing it out. It didn't occur to me that the zoom 1-3 tables were unbounded. I made the change to pull the `min_zoom` column into the zoom 4 generalized table and filter for `min_zoom< 7`.
With this change, here is Algeria, zoom 3:
Algeria, zoom 4:
Fixes#366
This PR sets `toll=1` in the `transportation` layer when a road is tagged as a toll road in OSM (with the tag `toll=yes`). If a road is tagged with any other value of `tag=*`, the value is suppressed in the tile, since non-toll roads can be presumed as the default and therefore this PR should have only negligible impact in the tiles.
Support for toll road tagging is of interest in the American mapping community, because toll roads have historically been styled differently on American-style maps, for example:
Screen shot of a toll road being generated in the `transportation` layer tiles:
Fixes#1066
This PR adds a new field `access` in the transportation layer, which will be set to `no` if the `access` tag is either `no` or `private`. While `private` is the more popular value by a 17:1 ratio, I went with `no` because it's smaller in the tiles. In addition, the text `no` opens up the future possibility of other text-based access values such as `destination`, `customers`, or `permit`.
The screenshot below shows an example of access tagging for a road on a military base:
This PR updates the `transportation` layer creation scripts to simplify the SQL and remove unneeded sub-selects, checks and conditionals, fix indexes, and improve inline documentation.
Currently, there are two sequences of materialized view creations. There is one from zoom 11 through 9, and a second one for zoom 8 through 4. This PR removes that break in the sequence of transportation table materialized view creations, in favor of one in which high-zoom views are created first, and then each lower zoom is created from the zoom above.
Instead, the current generalized zoom 8 transportation table is built directly from `osm_transportation_linestring` rather than being built from the zoom 9 transportation table. This means that when building the zoom 8 table, it must scan the entire transportation network rather than just selecting from the pre-filtered zoom 9 table.
This PR removes an unneeded sub-select in the build of the zoom 8 table, which appears to be a leftover from an old version of the SQL that did some sort of merge. Once this PR is implemented all zooms from 11 through 4 will be linked via a progressive series of materialized views.
Lastly, this adds in missing materialized view refreshes for zooms 9-11, which appear to have been entirely missing, and as far as I can tell aren't getting updated in the current version of this code.
In addition, the following optimizations were added as part of this commit:
1. Updates the `osm_highway_linestring_highway_partial_idx` partial index to match the `SELECT..WHERE` clause actually present in `transportation/update_route_member.sql`, which is where it appears to be actually used, and update inline documentation to reflect this.
2. Collapses unnecessary sub-select block in `osm_transportation_merge_linestring_gen_z11`, and removes unnecessary ST_IsValid() call, which already provided in `mapping.yaml`, and update inline documentation to reflect these assumptions.
3. Updates `WHERE` blocks to remove unnecesary checks and further document assumptions. The `highway=construction` check is unnecessary in all cases, because it is sufficient to check the `construction` key alone. If `construction=*` is set, then `highway=construction` is implied.
4. Two indexes were added to `layers/transportation/update_route_member.sql` to improve route population performance.
In testing locally, I'm seeing performance improvements around 10% in the generation of the `transportation` layer, based on modifying `openmaptiles.yaml` to generate only the transportation layer and then repeatedly running `time make import-sql`, however, this timing might be impacted by docker, so I would ask for confirmation of acceptable performance.
In addition, this PR shortens the length of the transportation update SQL file by 30 lines, which makes it easier for contributors to work with.
PR #1168 removed several `WHERE` clauses in the `transportation_merge_*` table series. With those removed, it appears that `osm_transportation_merge_linestring_gen_z8` and `osm_transportation_merge_linestring` are nearly identical, with the former simply adding an `ST_Simplify()` operation.
A grep of the codebase indicates that the _only_ use for `osm_transportation_merge_linestring` is to hold the intermediate result of an `ST_Dump(geometry))` before it is fed into `ST_Simplify()`. Therefore, it appears that we're holding an entire zoom 8 copy of the transportation layer (all motorway/trunk/primary roads) in a materialized view for absolutely no reason at all.
This PR removes the `osm_transportation_merge_linestring` intermediate table and changes the definition of `osm_transportation_merge_linestring_gen_z8` to perform the `ST_Dump()` and `ST_Simplify()` transformations directly from the `osm_highway_linestring` table in a single operation.
This PR removes several redundant/unnecessary WHERE clauses from the transportation layer. Specifically:
The table `osm_transportation_merge_linestring` is a view of `osm_highway_linestring` which exposes only motorway/trunk/primary roads:
9e4be3e3b0/layers/transportation/update_transportation_merge.sql (L122-L123)
However, the create statement for the table `osm_transportation_merge_linestring_gen_z8`, which is a view of `osm_transportation_merge_linestring`, also contains a `WHERE` clause which selects down to motorway/trunk/primary roads. This `WHERE` is unnecessary:
9e4be3e3b0/layers/transportation/update_transportation_merge.sql (L144-L145)
This unneeded `WHERE` clause is similarly present in the create statement for `osm_transportation_merge_linestring_gen_z7`, which is a view of `osm_transportation_merge_linestring_gen_z8`:
9e4be3e3b0/layers/transportation/update_transportation_merge.sql (L163-L164)
Likewise, there is a similar redundant `WHERE` clause in the `osm_transportation_merge_linestring_gen_z5` and `osm_transportation_merge_linestring_gen_z6` tables, both of which select down to `motorway` and `trunk`. This `WHERE` clause is only needed on the z6 table, and is redundant on the z5 table.
I am not sure what the performance penalty is for these redundant `WHERE` clauses, but there does not appear to be any reason to keep them, and they may incur a performance cost.
Changes in #1143 removed the need for `name` and `ref` parameters in the network-to-network_type conversion function. This PR removes this dead code.
Verified by running ./quickstart rhode-island which completed successfully.
This PR fixed bugs introduced in #1147 and #1119 which broke the update triggers in `transportation_name_update`. I noticed this issue while I was working a different PR and tried to update a table in the `transportation` layer.
**Test**
The following code currently fails because of cascading update failures, but will complete successfully after this PR is merged.
```
UPDATE osm_highway_linestring hl
SET network = rm.network_type
FROM osm_route_member rm
WHERE hl.osm_id=rm.member;
```
Fixes#1128
This PR adds 6 new columns to the `transportation_name` column, named `route_1` through `route_6`. These columns contain route information for a section of roadway. The value is stored in the form **network=ref**. For example, Interstate 95 in the United States would be `US:I=95`. Thus, each `route_N` value contains enough information to render a highway shield. Since a section of road can be a part of more than one route, the `route_2`, `route_3`, etc, will contain the 2nd, 3rd, etc., concurrent routes.
The technical approach was to extend the change in #1135, which added ordered concurrency indexes to the `osm_route_member` table by joining up to the the first six entries to the `osm_transportation_name_network` table. In addition, that PR provided a ranking system for concurrent highways, ordering first by `network_type` (for example, `us-interstate`, `us-state`, etc), then alphabetically by network name, and then by ref in ascending order. This ordering of concurrent route memberships is now exposed in this PR in the sequential `route_N` values, meaning that rendered concurrent highway shields will be reasonably sorted.
The renderings below were generated using this branch of OpenMapTiles, as well as a separate branch of openstreetmap-americana:
https://github.com/ZeLonewolf/openstreetmap-americana/tree/openmaptilers-new-features-test
The rendering approach is to use the [formatted expressions](https://maplibre.org/maplibre-gl-js-docs/style-spec/expressions/) feature in mapLibre to insert images into a string of text. Blank shields are added to the sprite sheet for all possible route networks. Next, a [styleimagemissing](https://maplibre.org/maplibre-gl-js-docs/api/map/) callback is registered. As each shield ID is requested, the callback retrieves the sprite shield blank associated with the route's network, draw the `ref` text on the shield, and insert the complete shield back into the map.
Of note, this approach currently results in shields which are rotated about the road rather than being viewport aligned. This issue is currently documented as maplibre/maplibre-gl-js#188. A separate repository (https://github.com/ZeLonewolf/maplibre-shield-rotation-sample) has been created as a test case to fix this rotation issue.
Adding route concurrency information to OpenMapTiles would be a major step forward in achieving comprehensive highway shield renderings in a vector map!
**Renderings**:
Fixes#1131
This change does the following:
1. Excludes roads from `transportation_name` that don't have a `name` or a `ref`
2. Updates the road name merging logic to exclude changes in `brunnel` status. This will ensure that minor bridges don't disrupt the continuity of named roads as the map zooms out.
3. The `brunnel` tag will now only be set when a bridge or tunnel is distinctly named. Distinctly named is defined as "has a different name from the road on either side".
This example shows an unnamed interstate highway rendered as a continuous feature at low zoom. This road has many small bridges along its length:
This example shows a named bridge rendered with `brunnel` tag set:
Brian Sperlongano
