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.

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

Resolve wrong layering of low-zoom layers #555

Closed

Conversation

matthijsmelissen
Copy link
Collaborator

The rendering rules of the low-zoom layer are inserted into the regular
layers. This makes sure that low-zoom roads are rendered like other
roads, and prevents therefore incorrect layering of roads on low zoom
levels.

It should also improve the readability of the code.

This solves #400.

This supersedes #493.

The rendering rules of the low-zoom layer are inserted into the regular
layers. This makes sure that low-zoom roads are rendered like other
roads, and prevents therefore incorrect layering of roads on low zoom
levels.

It should also improve the readability of the code.

This solves gravitystorm#400.
@gravitystorm
Copy link
Owner

I suspect this would be a significant performance problem. The lowzoom layer was coming from the planet_osm_roads table, whereas selecting the motorways out of the lines table at z5 would take a lot longer.

As for ordering, I'm fine with 'importance'-based ordering at low zoom levels and 'physical' ordering at high zoom levels, if that makes things easier.

Thoughts?

@matthijsmelissen
Copy link
Collaborator Author

I suspect this would be a significant performance problem. The lowzoom layer was coming from the planet_osm_roads table, whereas selecting the motorways out of the lines table at z5 would take a lot longer.

Thanks, I missed the roads/lines difference. I will test the performance first.

As for ordering, I'm fine with 'importance'-based ordering at low zoom levels and 'physical' ordering at high zoom levels, if that makes things easier.

Yes, makes sense.

Closed for now.

@matthijsmelissen matthijsmelissen deleted the lowzoom2 branch July 20, 2014 13:32
@matthijsmelissen
Copy link
Collaborator Author

Can I find anywhere what the difference between planet_osm_roads an planet_osm_line is?

@pnorman
Copy link
Collaborator

pnorman commented Aug 4, 2014

Can I find anywhere what the difference between planet_osm_roads an planet_osm_line is?

The C tag transform code. The default lua transform might be an easier read and should be equivalent.

@matthijsmelissen
Copy link
Collaborator Author

If I read it correctly, the tags in planet_osm_roads are:

highway = {secondary_link, secondary, primary_link, primary, trunk_link, trunk, motorway_link, motorway}, railway=*, boundary=administrative.

Although I don't understand why tagtransform.c checks for

if (boundary && !strcmp(boundary, "administrative"))

rather than

if (boundary && strcmp(boundary, "administrative"))

@gravitystorm
Copy link
Owner

that's just the way strcmp works - it returns 0 if they are equal so you need to negate it to make it true.

@matthijsmelissen
Copy link
Collaborator Author

Ah, thanks.

matthijsmelissen added a commit to matthijsmelissen/openstreetmap-carto that referenced this pull request Jan 24, 2015
This merges code for low-zoom road definitions into the corresponding
high-zoom road definitions.

Data on z<10 now comes from osm_planet_roads, data on z>=10 comes from
osm_planet_line. Chosen is for zoom level 10 as cut-off, because residential
is rendered from z10 and is not included in osm_planet_roads, so on z>=10
osm_planet_line needs to be queried anyway.

Advantages:
* This separates the zoomlevel cut-off for querying from the roads vs line
  table from the zoomlevel cut-off for the lowzoom rendering style.
* This brings related code (i.e. code per feature) closer together in the file,
  which should make the code more easy to modify.

This is mainly a code cleanup, but it causes some small changes in rendering:
* Take into account layering of roads on z10 and z11.
* Fix the loss of layering problem of railway tunnels on z12 (resolves gravitystorm#400).
* Fix bug that prevented tunnel rendering style for railway on z11/12, and add
  tunnel style rendering for spur/siding/yard for z<12.
* Give railways on z<12 a round line-join.

This is an improvement of gravitystorm#555.
matthijsmelissen added a commit to matthijsmelissen/openstreetmap-carto that referenced this pull request Jan 24, 2015
This merges code for low-zoom road definitions into the corresponding
high-zoom road definitions.

Data on z<10 now comes from osm_planet_roads, data on z>=10 comes from
osm_planet_line. Chosen is for zoom level 10 as cut-off, because residential
is rendered from z10 and is not included in osm_planet_roads, so on z>=10
osm_planet_line needs to be queried anyway.

Advantages:
* This separates the zoomlevel cut-off for querying from the roads vs line
  table from the zoomlevel cut-off for the lowzoom rendering style.
* This brings related code (i.e. code per feature) closer together in the file,
  which should make the code more easy to modify.

This is mainly a code cleanup, but it causes some small changes in rendering:
* Take into account layering of roads on z10 and z11.
* Fix the loss of layering problem of railway tunnels on z12 (resolves gravitystorm#400).
* Fix bug that prevented tunnel rendering style for railway on z11/12, and add
  tunnel style rendering for spur/siding/yard for z<12.
* Give railways on z<12 a round line-join.

This is an improvement of gravitystorm#555.
matthijsmelissen added a commit to matthijsmelissen/openstreetmap-carto that referenced this pull request Feb 8, 2015
This merges code for low-zoom road definitions into the corresponding
high-zoom road definitions.

Data on z<10 now comes from osm_planet_roads, data on z>=10 comes from
osm_planet_line. Chosen is for zoom level 10 as cut-off, because residential
is rendered from z10 and is not included in osm_planet_roads, so on z>=10
osm_planet_line needs to be queried anyway.

Advantages:
* This separates the zoomlevel cut-off for querying from the roads vs line
  table from the zoomlevel cut-off for the lowzoom rendering style.
* This brings related code (i.e. code per feature) closer together in the file,
  which should make the code more easy to modify.

This is mainly a code cleanup, but it causes some small changes in rendering:
* Take into account layering of roads on z10 and z11.
* Fix the loss of layering problem of railway tunnels on z12 (resolves gravitystorm#400).
* Fix bug that prevented tunnel rendering style for railway on z11/12, and add
  tunnel style rendering for spur/siding/yard for z<12.
* Give railways on z<12 a round line-join.

This is an improvement of gravitystorm#555.
matthijsmelissen added a commit to matthijsmelissen/openstreetmap-carto that referenced this pull request Feb 9, 2015
This merges code for low-zoom road definitions into the corresponding
high-zoom road definitions.

Data on z<10 now comes from osm_planet_roads, data on z>=10 comes from
osm_planet_line. Chosen is for zoom level 10 as cut-off, because residential
is rendered from z10 and is not included in osm_planet_roads, so on z>=10
osm_planet_line needs to be queried anyway.

Advantages:
* This separates the zoomlevel cut-off for querying from the roads vs line
  table from the zoomlevel cut-off for the lowzoom rendering style.
* This brings related code (i.e. code per feature) closer together in the file,
  which should make the code more easy to modify.

This is mainly a code cleanup, but it causes some small changes in rendering:
* Take into account layering of roads on z10 and z11.
* Fix the loss of layering problem of railway tunnels on z12 (resolves gravitystorm#400).
* Fix bug that prevented tunnel rendering style for railway on z11/12, and add
  tunnel style rendering for spur/siding/yard for z<12.
* Give railways on z<12 a round line-join.

This is an improvement of gravitystorm#555.
jeisenbe added a commit to jeisenbe/openstreetmap-carto that referenced this pull request Nov 7, 2018
jeisenbe added a commit to jeisenbe/openstreetmap-carto that referenced this pull request Nov 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants