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

Render barrier=jersey_barrier #4923

Merged
merged 2 commits into from
Jan 21, 2024

Conversation

dch0ph
Copy link
Contributor

@dch0ph dch0ph commented Jan 10, 2024

Fixes #4854

Changes proposed in this pull request:
barrier=jersey_barrier on node is rendered as barrier=block/log .
barrier=jersey_barrier on way is rendered as default barrier, i.e. same as wall, fence.

There doesn't seem any value in introducing a distinct signature for jersey barrier as a way; the point is that the blocks form a continuous barrier. Although personally there doesn't seem much point in even distinguishing a jersey barrier used as a block, from an "ordinary" block, there would be even less point in rendering them differently.

Test rendering with links to the example places:

Node

Current
image

After
image

Way

image

image

(Example of barrier between carriageways is not the prettiest, but nearest to hand and makes the point)

barrier=jersey_barrier on node is rendered as barrier=block/log
barrier=jersey_barrier on way is rendered as default barrier, i.e. same as wall, fence
@imagico
Copy link
Collaborator

imagico commented Jan 11, 2024

Thanks for the pull request.

I agree rendering this with the generic line signature for linear barriers is the best approach. If we should decide to differentiate barrier rendering by type of barrier we should first do this for the main linear barrier classes (fence/wall/retaining_wall).

I am not sure if rendering nodes with barrier=jersey_barrier is a good idea. This mapping concept is ambiguous in meaning - it is not clear if a node with that tag on a road physically blocks the road for all modes of transport (like a barrier=fence/barrier=wall across the road) or if it just prevents passage of larger vehicles (like barrier=block) or if it just constrains passage so it can easily be blocked completely with a single parked vehicle.

As a general note: Please always post examples in native resolution.

@dch0ph
Copy link
Contributor Author

dch0ph commented Jan 11, 2024

I'm inclined to agree about nodes with barrier=jersey_barrier. The wiki implies its use on a node corresponds to access=no, effectively a line of blocks across a road, and so it is arguably misleading to render in the same way as barrier=block. But I suspect that many mappers take the view, as implied in the original issue from @ppete2, that barrier=jersey_barrier on a node corresponds to a single block preventing just vehicles.

Personally I err to a more maximalist view of "showing something for the user", but I appreciate that OSMCarto takes a more conservative "only show what is well defined" line.

I'll leave things for comment for a week or so.

As a general note: Please always post examples in native resolution.

Apologies. My test render chain only outputs PDF via nic4.py. But I doubt there's any ambiguity over whether the PR works.

P.S. Although if we are not rendering barrier=jersey_barrier on nodes on the basis that it is poorly defined, we should surely stop rendering the dumb tag barrier=log!

@imagico
Copy link
Collaborator

imagico commented Jan 11, 2024

I'll leave things for comment for a week or so.

Yes, input on the matter of if use of barrier=jersey_barrier is semantically well defined in practical use would be welcome.

But I doubt there's any ambiguity over whether the PR works.

The purpose of examples is not primarily to serve as a test for correct operation, it is to allow people following this issue tracker to quickly get an idea how a change will look like in the map.

For this change this is not a big deal because evidently it will look like the other barrier=* features already rendered. But as a general principle it is good to have a native resolution test.

P.S. Although if we are not rendering barrier=jersey_barrier on nodes on the basis that it is poorly defined, we should surely stop rendering the dumb tag barrier=log!

That is a separate issue - but i would not call barrier=log a dumb tag. The problem is more if it is well enough established and consistently used for what we render it as. barrier=log rendering was added in #3054 at a time when we had no consensus requirement on changes. There is the competing tag obstacle=fallen_tree - which is semantically much more clearly defined but less widespread in use.

Remove rendering of jersey_barrier as node
@dch0ph
Copy link
Contributor Author

dch0ph commented Jan 18, 2024

In the absence of further comments, I've removed the node rendering, leaving a fairly trivial change to render a way as a default barrier.

Can you squash commits as part of the merge? I couldn't work out how to do this easily.

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.

Works as intended, rendering barrier=jersey_barrier on ways just like all the other simple barriers (wall, fence etc.) with a simple gray line at z16+.

@imagico imagico merged commit 9fc6ed4 into gravitystorm:master Jan 21, 2024
2 checks passed
@dch0ph dch0ph deleted the render-jersey-barrier branch January 27, 2024 11:28
@imagico imagico mentioned this pull request Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Render barrier=jersey_barrier
2 participants