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

Jump to bottom

Road casings ordering leads to "bridge" like rendering #42

Closed
Edefritz opened this issue Jun 12, 2023 · 6 comments
Closed

Road casings ordering leads to "bridge" like rendering #42

Edefritz opened this issue Jun 12, 2023 · 6 comments

Comments

@Edefritz
Copy link
Contributor

Right now all road casings are rendered just before the road type they belong to. This leads to some weird effects at intersections between different road types (minor-medium/medium-major ...), where it appears that the higher class road passes above the lower class road, instead of being just a regular intersection.

Screenshot 2023-06-12 at 11 13 25 Screenshot 2023-06-12 at 11 13 38

I'm not a cartography expert, but I think road casings should all be rendered before (below) any of the roads. I will propose a PR later today, if that's ok.
Edefritz added a commit to Edefritz/basemaps that referenced this issue Jun 12, 2023
@Edefritz
Place road casings below all roads to prevent "bridge" like rendering…
315990c 
… effect of regular intersections.

See protomaps#42
@Edefritz Edefritz mentioned this issue Jun 12, 2023
Merged
@nvkelso
Copy link
Collaborator

nvkelso commented Jun 12, 2023

+1

bdon pushed a commit that referenced this issue Jun 23, 2023
@Edefritz
Place road casings below all roads to prevent "bridge" like rendering…
3557121 
… effect of regular intersections. (#43)

See #42
bdon added a commit that referenced this issue Jun 24, 2023
@bdon
Re-order default style to correct casings for bridges and tunnels [#42]
8cd6739
@wipfli
Copy link
Collaborator

wipfli commented Jun 25, 2023

Thanks for this contribution, @Edefritz! I agree that in general it is better to do first all casings, then all line fills for roads.

Nevertheless, I would like to mention that there are also downsides to first render all casings, and then all line fills:

  1. Roads of lower priority which merge perpendicularly into a road with higher priority will "bite out" a section of the casing of the high priority road which breaks the visual continuity of the high priority road. This effect is in particular visible at low zooms.
  2. A low priority road parallel and close to a high priority road will delete the casing of the high priority road. It looks effectively like the high priority road has no casing. Again, this effect is strongest at low zooms.

Here is an example from the Wikipedia basemap which has these problems (https://maps.wikimedia.org/#15/47.2109/7.9993):

Screenshot_20230625_214632

Google Maps solves this issue by using casing-fill-casing-fill order at low zooms, while switching to casing-casing-fill-fill at higher zooms. Here is an example:

At z13:

https://www.google.com/maps/@47.2076993,8.0005265,13z?entry=ttu

image

At z14:

https://www.google.com/maps/@47.2076993,8.0005265,14z?entry=ttu

image

Side Note: Bridges

The visual continuity of roads in Google Maps is something which sets it apart from other basemaps in my opinion. This does not only affect casing ordering, but also the z-ordering of roads (bridges/tunnels). At low zooms, they show high priority roads on top of low priority roads, independent of the physical ordering. highway=motorway will always shown on top of highway=primary at low zooms. Only once you zoom in enough, the physical ordering is actually shown.

Example

At z15:

https://www.google.com/maps/@47.2095108,7.9834262,15z?entry=ttu

Screenshot_20230625_220113

At z16:

https://www.google.com/maps/@47.2095108,7.9834262,16z?entry=ttu

Screenshot_20230625_220126

@Edefritz
Copy link
Contributor Author

Thanks for the explanation @wipfli! I wasn't aware about all the nuances when it comes to casings and suspected a simple bug.
I guess now that my MR was accepted we can close that issue as soon as a new version of the npm package is released.

bdon added a commit that referenced this issue Jul 12, 2023
@bdon
Re-order default style to correct casings for bridges and tunnels [#42]…
7a0f950 
… (#50)
@bdon
Copy link
Member

bdon commented Jul 12, 2023

#58

please note that right now the styles and tiles on main are out of sync. the styles will work with v2 tiles, which are the ones on api.protomaps.com/tiles/v2 but HEAD is currently on v3. In the future we will unify both tiles and styles under a single semantic versioning scheme, but this is a couple months away.

nvkelso added a commit to nvkelso/basemaps that referenced this issue Jul 12, 2023
@nvkelso
similar casing order change as protomaps#42 for merge conflicts
90f584b
nvkelso added a commit to nvkelso/basemaps that referenced this issue Jul 12, 2023
@nvkelso
reserve merge conflict over road casing order protomaps#42
49e81d4
@bdon
Copy link
Member

bdon commented Jul 20, 2023

1.3.1 on npm: https://www.npmjs.com/package/protomaps-themes-base

bdon added a commit that referenced this issue Jul 20, 2023
@nvkelso @bdon
Round 2 enhancements for Felt (#47)
50d0aa7 
* bug fix for town, village as city level features (not neighbourhood)

* init Makefile targets

* add pmap:kind coallesce

* converge towards shield_text_length; pass thru other network values; set pmap:kind_detail on other kind

* refactor pmap:kind calculation and add pmap:kind_detail

* refactor places, add pmap:min_zoom, add sort func, add label grid

* rework low zoom NE water

* stub out water label points (natural earth only)

* fix test tile coord; tmp pmtiles bin location

* add kind = national_park logic

* add derived water polygon labels in physical points layer

* fix java errors, comment out setSortKey for later debug

* reorder sections for a-z legibility, add optional pmap:kind_detail property coallese and extra tags (#39), show some POIs earlier depending on kind or area, special handling for kind = national_park

* add washington state target

* show county lines earlier, export Tilezen kinds (and region not state)

* block {building|building:part}=no; push building:part to later zooms; merge buildings at z14; note future height quantization

* do NOT export names; formatting

* do NOT export names, or areas

* export label points for water polygons; & etc

* use Tilezen kind values (locality not city)

* export POIs for ways, rels too (area graded per zoom); some kind of boundary and landuse; national_park kind; add kind_detail; special case no name POI zooms; special case other minor kind zooms

* fix service minor roads; fix sidewalks

* show piers later; drop short featues

* fix NE layer parsing; do NOT export names (see physical points layer instead)

* quantize heights by zoom; push building_part to later zooms

* always export pmap:kind

* add national_park, protected_area, and nature_reserve (moved from natural layer); show at earlier zooms; more sophisticated kind setting; merge features at early zooms

* remove national_park, protected_area, nature_reserve (moved to landuse layer); merge features at early zooms

* show rivers earlier; always set kind value; intermittent to boolean

* switch to min_label for NE instead of min_zoom; update zoom area grading; export kind_detail

* prefix population_rank with pmap:

* bug fixes for kind calculations; better way area calculation; show larger area features earlier by area and zoom and kind; show taller height kinds earlier; export min_zoom for label collision

* show motorway and primary earlier; show service roads later; show paths later; do not remove links early

* show airport runway, taxiway earlier; show some types of rail later; smarter kind calculation

* update kind calculation; boolean exports

* prefix population_rank with pmap:

* remove debug attr

* add phony list; remove complicated targets

* add beach, forest, military, naval_base, airfield, zoo kinds; bug fix national_park; reorder poly var creation later; stop merging tile features

* move kind calc above; Tilezen schema notes; min pixel size to 2 from 3

* some tiny lake labels earlier; add Tilezen schema comments

* refactor property gathering logic and move setAttr later; add Tilezen schema notes

* add attraction kinds, beach kind, hide early node university kind, add (national) forest kind, bug fix for national_park; add few tier 1 landuse kinds to early zoom allowlist; fix height zoom grading bug; hide mid-zoom hotels (from height); Tilezen schema notes

* refactor property gathering and attr setting; Tilezen schema notes

* Tilezen schema notes; hide names from early zooms for more merging

* refactor property gathering and attr setting; Tilezen schema notes

* remove unused imports, fix silly Java error

* add to early zoom blocklist

* update style for schema changes

* add SF target

* ensure pier pass thru

* add grass

* add label grid, separate NE from OSM at zoom 7

* add label grid; bug fixes for kinds and zooms (per kind)

* add planet target

* add new targets

* use low zoom boundaries from Natural Earth

* add zoom logic for country, region

* guard against nulls

* guard against null names; spell out more country names

* guard against null names

* perf; rework NE kind and kind_detail

* npe, again; cleanup imports and var names

* standardize on var names

* ensure odd numbered admin_level do not export

* add brunnel pmap:level, remove dedicated props

* add brunnel; push disused lines later

* add quarter back

* standardize sf var name

* push small parks later; university buildings later

* brunnel; standardize sf var name; kind_detail

* regrade park area > zooms

* add changelog, version, and semver statement

* similar casing order change as #42 for merge conflicts

* update 1.0.0 date

---------

Co-authored-by: Brandon Liu <[email protected]>
@Edefritz
Copy link
Contributor Author

Thanks so much. I will close the issue then :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@bdon @nvkelso @Edefritz @wipfli