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

Additional access marking #5049

Merged
merged 4 commits into from
Jan 16, 2025

Conversation

dch0ph
Copy link
Contributor

@dch0ph dch0ph commented Dec 30, 2024

Fixes #5024
Fixes #4998

Changes proposed in this pull request:

  • Render "restricted" access (e.g. access=destination) on highway=motorway/trunk/primary (currently only access=no shown)
  • Add access rendering for highway=pedestrian using white as contrasting colour.

Note that a tweak to functions.sql is needed to allow highway=pedestrian to support "restricted" access marking.

Test rendering:

"restricted" access on major highways is not expected to be valid tagging. The purpose of the PR is to highlight potentially incorrect tagging.

Illustrations are taken from a hacked legend rendering, with access=destination on all highways.

image

image

Note that the increased "coverage" of access tags needs to be weighed against a 7% increase in lines of XML. This is a result of all the added combinations of road types / 3 SQL queries / 3 access types.

Copy link
Collaborator

@imagico imagico left a comment

Choose a reason for hiding this comment

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

This looks good - except for the color of the dashing for pedestrian roads IMO - using white here breaks with the overall principle of using gray colors for the access dashing on roads. It also can lead to confusion with the construction dashing, which uses white color as well. Furthermore it is much higher contrast than the other roads relative to the fairly dark pedestrian fill color.

I would probably try the light gray used for highway=road access dashing - the gray fill color of highway=road has similar contrast to that relative to highway=pedestrian - and that would more or less also match the contrast of the dashing on other road types.

new

As far as the increase in XML code volume is concerned - that is to be expected. This is an unfortunate side effect of how our road rendering is designed. It is important to keep an eye on this - but i am very much against constraining our design decisions because of such effects. Ways exist to counteract this code volume increase (by moving some of the styling logic outside of CartoCSS/XML - like we did for example with the path to footway/cycleway/bridleway re-classification). But we need to carefully weigh the pros and cons here (code volume, rendering efficiency and code accessibility and maintainability).

Change pedestrian access marking colour to common light shade of grey.
Also consolidate names.
@dch0ph
Copy link
Contributor Author

dch0ph commented Jan 14, 2025

I would probably try the light gray used for highway=road access dashing - the gray fill color of highway=road has similar contrast to that relative to highway=pedestrian - and that would more or less also match the contrast of the dashing on other road types.

Changed as suggested. I've also consolidated the variable names for ease of maintenance - the access marking on pedestrian, primary and road is the same light colour for the same reason.

Copy link
Collaborator

@imagico imagico left a comment

Choose a reason for hiding this comment

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

Thanks. Looks better now:

new

@imagico imagico merged commit 248b72d into gravitystorm:master Jan 16, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants