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

[WIP] Rewrite roads layers #2869

Closed
wants to merge 29 commits into from
Closed

[WIP] Rewrite roads layers #2869

wants to merge 29 commits into from

Conversation

pnorman
Copy link
Collaborator

@pnorman pnorman commented Sep 26, 2017

Fixes #2046
Fixes #2468

This is a complete rewrite of the transportation code. The primary goals are

  • Go from 9 road layers to 2
  • Get all the MSS for each feature in the same place
  • Avoid selecting from planet_osm_line once for roads and once for railways
  • Reduce MSS duplication

Once I'm farther along I'll set up a demo, but there are still features not yet implemented.

The layer changes are to move barrier-like features, landuse overlays, and ferry-routes before roads, and to move aerialways and waterway-bridges after roads.

Cartographic changes

  • Non-motorized paths (footway/path, cycleway, and bridleway) use the footway width
  • Various minor adjustments of start zooms for casings, bridges, etc to be consistent.

Remaining work

  • More railways
  • Tunnels
  • Oneway arrows
  • Access
  • Low zoom roads
  • End caps
  • Unify symbolizer names

Long-term I'd like to redo names, but intend to keep that out of this PR

Known issues

  • Footway weight has changed.

I'm not sure what has caused this, but unknown surface footways are lighter.

image

For testing, areas likely to have problems will involve

  • Endcaps
  • Complex layering

@pnorman pnorman added the roads label Sep 26, 2017
}
}
}
[highway = 'pedestrian'],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing newline above this line

project.mml Outdated
AND points.highway IN ('turning_circle', 'turning_loop')
AND lines.highway IN ('tertiary', 'unclassified', 'residential', 'living_street', 'service', 'track')
ORDER BY points.osm_id, -- for DISTINCT ON
CASE lines.highway -- Use the most important type for cases where the TC is at an intersection. TODO: Should lines.layer be in here?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it possible to simply use z_order instead of this multiline-CASE?

project.mml Outdated
COALESCE(bridge IN ('yes', 'boardwalk', 'cantilever', 'covered', 'low_water_crossing', 'movable', 'trestle', 'viaduct'), FALSE) AS bridge,
COALESCE(layer,0) AS layernotnull,
z_order,
surface IN ('paved', 'asphalt', 'cobblestone', 'cobblestone:flattened', 'sett', 'concrete', 'concrete:lanes', 'concrete:plates', 'paving_stones', 'metal', 'wood')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Doesn’t this query always return TRUE for everything that is not in the “unpaved” list anyway, making the query for th e“paved” list obsolete?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, but there is a bug with values not in either list.

@pnorman
Copy link
Collaborator Author

pnorman commented Sep 29, 2017

Noting a minor bug

Pedestrian features are rendering above turning circles fill

image

@sommerluk
Copy link
Collaborator

Currently, first comes the code for more important features with high z_order (e.g. highway=motorway) and later the code for less important features with low z_order (e.g. highway=track).

Might it be an option to use the opposite order?

This would not make any functional change.

For the code proposed in #2640 (unpaved roads with dst-out pattern), the opposite order would make it a lot easier. In fact, the diff of #2640 is difficult to read because it needs the opposite order because it uses attachments. The code changes themself are not so heavy.

And maybe this is also useful independent from #2640. Indeed the old road code has high z_order features first. But the attachment concept in MSS works the other way around: First attachment drawn first, last attachment drawn last (above the other ones). As we use attachments anyway in our roads MSS code, it could be more intuitive to have the rest following the same logic…

Repository owner deleted a comment from sommerluk Oct 12, 2017
@pnorman
Copy link
Collaborator Author

pnorman commented Oct 13, 2017

Currently, first comes the code for more important features with high z_order (e.g. highway=motorway) and later the code for less important features with low z_order (e.g. highway=track).

Might it be an option to use the opposite order?

I find it easier to read from most important to least important.

For the code proposed in #2640 (unpaved roads with dst-out pattern), the opposite order would make it a lot easier. In fact, the diff of #2640 is difficult to read because it needs the opposite order because it uses attachments. The code changes themself are not so heavy.

I'd rather keep it as-is for now, then we can look at changing it if necessary.

As we use attachments anyway in our roads MSS code, it could be more intuitive to have the rest following the same logic…

Generally we order in descending significance for MSS.

It's interesting to note that within the SQL, the order by statements are both ascending and descending, depending on if its drawing lines and fills, or placing labels.

@sommerluk
Copy link
Collaborator

It's interesting to note that within the SQL, the order by statements are both ascending and descending, depending on if its drawing lines and fills, or placing labels.

Good point.

@sommerluk
Copy link
Collaborator

Will the road layer rewrite resolve at the same time #2595 (construction roads should not be wider than their finished counterparts)?

@pnorman
Copy link
Collaborator Author

pnorman commented Nov 4, 2017

Will the road layer rewrite resolve at the same time #2595 (construction roads should not be wider than their finished counterparts)?

I'm not intending to make cartographic changes, but that one will depend on how the construction MSS is structured.

@pnorman pnorman mentioned this pull request Dec 4, 2017
@kocio-pl
Copy link
Collaborator

kocio-pl commented Jan 16, 2018

@pnorman What is the current state of this code? Do you still plan to update it to be mergable (maybe just some part of it, if the task is too big)?

@pnorman
Copy link
Collaborator Author

pnorman commented Feb 21, 2018

@pnorman What is the current state of this code?

I've been lacking time and interest.

Do you still plan to update it to be mergable (maybe just some part of it, if the task is too big)?

I don't really see a way to split it down in scope.

The best way to help this out would be for someone to pair up with me and we could work on it together. This would also be a good chance for someone to learn more about complex CartoCSS. The new roads code is simpler, but roads are still one of our biggest areas.

@kocio-pl
Copy link
Collaborator

Thanks for the information. Is there anybody wanting to help Paul with this code?

@matthijsmelissen
Copy link
Collaborator

I'm closing this as currently nobody is interested in working on this.

If anybody is interested, feel free to create a new proposal.

@pnorman Thanks a lot for the work you put in so far! Hopefully somebody else can pick up.

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.

4 participants