-
Notifications
You must be signed in to change notification settings - Fork 821
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 highway=busway #4456
base: master
Are you sure you want to change the base?
Render highway=busway #4456
Conversation
Thanks for working on this. I made some more general comments on the idea of rendering highway=busway in #4226. Some quick comments on the concrete changes from a first glance:
|
This PR is not perfect, but even as an interim solution it is a major improvement over not rendering anything. I am willing to refine the styling at a later point in time when the community has had a chance to give feedback based on its effect on the rendered map, but without such feedback questions about whether the colours work or not are hard to answer without falling back to assumptions rather than practical use-cases. Let's start of simple and introduce improvements gradually. |
Well - we have had this discussion plenty of times in the past. Different maintainers here have different approaches to map design. But we have no consensus on primacy of feature addition over the long term sustainability and the documented goals of the project. My comments are meant to provide guidance that can help you to develop this into a state that finds consensus among the maintainers to be accepted and that supports the goals of the project. Among other things that involves choosing colors in a way that does not cause confusion or otherwise clashes with the map readability goal. Following the semantic homogeneity principle is usually a good idea (features similar in meaning and purpose for the target map user should use similar colors while features different in meaning and purpose should differ more in coloring). The promise of working on iterative improvements in the future - as admirable as that is - will not lead us to accept design debt or technical debt in changes when it can be avoided. Designing feature additions in a style as rich and complex as this without compromising map readability and constructive mapper feedback is hard. But it is doable if you are willing to think outside the box and take into account the cartographic context and make adjustments to that as needed - a holistic approach to design. That is why i suggest in #4226 to take into account highway=bus_guideway as well. |
@imagico Feel free to suggest different colours for:
I can use the current blue of
Thinking outside the box, this seems like a sensible timeline to me:
In the mean time, if #4395 can be reviewed, I can use the outcome of that in this PR as well. This PR is already a huge improvement over not rendering anything, so let the final 5% of it come in due time — start with the 95% of a quick-win. This whole project is one of iterative improvements after all (and that is a good thing). |
95eebbf
to
2455ab9
Compare
For comparison, here the colours currently used by diff --git a/style/roads.mss b/style/roads.mss
index 815a474..a4fa6c4 100644
--- a/style/roads.mss
+++ b/style/roads.mss
@@ -3,7 +3,7 @@
@tertiary-fill: #ffffff;
@residential-fill: #ffffff;
@service-fill: @residential-fill;
-@busway-fill: #f2eef9;
+@busway-fill: #e5eeff;
@living-street-fill: #ededed;
@pedestrian-fill: #dddde8;
@raceway-fill: pink;
@@ -24,14 +24,14 @@
@helipad-fill: @aeroway-fill;
@access-marking: #eaeaea;
@access-marking-living-street: #cccccc;
-@access-marking-busway: #cbbce6;
+@access-marking-busway: #8fb4ff;
@default-casing: white;
@tertiary-casing: #8f8f8f;
@residential-casing: #bbb;
@road-casing: @residential-casing;
@service-casing: @residential-casing;
-@busway-casing: #b099d9;
+@busway-casing: #6699ff;
@living-street-casing: @residential-casing;
@pedestrian-casing: #999;
@path-casing: @default-casing; |
fed0f06
to
f001d3a
Compare
The z-order now sits below the Depending on the strictness of the tag's documentation, it is possible that this may be need to be re-evaluated at some point if this roads are treated as more higher level road types, but for now staying close to |
Note that fully implementing this PR would require a reload of the database for any implementations of this map style, including on openstreetmap.org - openstreetmap-carto.lua is edited to change the z-order and this happens when the rendering database is loaded. In the past we have usually waited to merge changes like this until a major version change, which is timed to happen when the openstreetmap.org rendering servers will be updated. This happens once a year or so, historically. |
@jeisenbe Any indication on when the next non-database-reload update and the next normal update will be? Landing this feature without the z-order line (postponing that for the next database reload) could be considered, although it is not ideal: Depending on the timescale, it may be preferable to not rendering this feature though. If it takes another 10 months I would recommend considering that interim approach. If it's foreseen to happen soon that's not necessary of course. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested this change now a bit and for better understanding of my comments on the color context of this as noted in #4456 (comment) - here an illustration of the color context of this change within the rest of the style:
I like to emphasize that it is not that the chosen road fill color cannot work in absolute terms. But within the design context we have here with other purple and surrounding colors used for semantically completely different things it does not make much sense. That is not primarily a fault of this PR but the collective result of many non-systematic color choices (see #2270). The problem with that is that two or three further color choices of this kind (finding a free spot in color space and looks good to me in the few simple cases i tested) and we end up with a color palette so screwed and intertwined that the style will be unmaintainable and the only choice a designer would have is to completely rebuild the style color wise more or less from ground up.
As before i am not going to oppose this change if others see it fitting into a sustainable development of this style. But i like to warn with emphasis that we would be digging ourselves further into a hole design wise accumulating more and more design debt if we keep adding fairly arbitrarily chosen colors that we already know to cause problem when added and don't make any move towards looking at the larger picture and addressing #2270.
Independent of that the following things do not work for me as is:
- dedicated support for tagging
highway=busway
+access=yes
- that tagging makes about as much sense as taggingbuilding=*
+demolished=yes
ornatural=wood
+trees=no
. The whole premise of the highway=busway tag is that it is for roads exclusive to buses. If it is not exclusive and just happens to be also used by buses it is evidently to be tagged with a normal road tag. - access restriction dashing in this style is by choice in gray color tones - we deliberately moved to that from the previous pink dashing. This change would depart from that choice by introducing dashing in a color related to the road fill color. Such a move could be discussed but it would need to be done collectively for all road classes and not for busways in isolation and it definitely should be a separate change.
I'm not unsympathetic to your plea, but #2270 was opened over five years ago. Freezing the addition of rendering new (approved and in use!) features until that is resolved would amount to keeping issues like #4226 open indefinitely. I strongly recommend resolving such issues in parallel with maintaining the current feature set. It's totally fine too completely change the styling of
If tagging with
Here it is in grey. I don't think it looks as good, especially given that this highway type is nearly always rendered with it. What do others think? Same hue as the way itself on the left, grey on the right. |
We don't render The analogy you should be looking at is |
f001d3a
to
e513ba9
Compare
I understand your point of view, but I feel it would be wrong for this style to unilaterally declare I also find that not defaulting to
But, if this point will prevent this PR from landing, this small commit which defaults |
@imagico this PR is now idle for two months; two months without rendering of busways. Note that Carto lagging behind OSM on something important like a highway tag causes trouble for Carto users (like public transport companies/passengers), with the risk of making Carto a less relevant renderer. Please consider speeding up this PR. |
This does not depend on me at the moment - i expressed above in my review what things do not work for me as is (i.e. what would need to change before i could support merging this PR) and what i dislike and would recommend against although i would be fine with in case other maintainers support it. And again: Everyone is welcome to review PRs and comment on the changes - this is not something that only maintainers can do. So far no one other than @jdhoek and me has contributed to the discussion on this change in substance and other perspectives would definitely be valuable. I suggest also to take into account the discussion on #4226, in particular my comments in #4226 (comment) and #4226 (comment). From what i can see no one has so far done any work on better documentation of actual use of tags in the domain, documenting if and how various maps render the different tags or on developing consensus in the mapper community on the fundamental semantic issues with the highway=busway tagging and how the different taggings are to be delineated in mapping in light of this. Side note: I have some time ago looked at options how to better and more intuitively represent the different modes in road classification in OSM (i.e. the classical road class hierarchy, the physical characterization and the access restrictions/primary modes of transport). Here in this style there are serious obstacles to that (in particular the annoying use of blue for cycleways) but in principle there are options that can accomplish this. But it requires looking beyond individual atomic feature additions and looking at the larger picture. I have not gotten around publishing that work yet though. |
I think I addressed these with the variants provided. Those only change a line here or there though. The bulk of the work is in 1b60743, which can serve as the basis of any refinement. Any maintainer who wishes to merge this feature can cherry-pick those optional commits and for the final tweaks. I don't recommend them, but the choice isn't mine.
Who does it depend on? Is there a shortage of active maintainers? This is a feature that deserves at least a first iteration of rendering, but none of the maintainers are stepping up to merge it or provide their own implementation. Nobody expects instant gratification, but this is getting to the point of stagnation. This worries me, because Carto is a beautiful style with a very high responsibility placed upon it by virtue of being the face of OpenStreetMap. As I said above:
|
To again explain the fundamental way this project works: Developers make pull request with changes they would like to be merged. Everyone is invited to review those changes. The developers making the pull request adjust their work based on the discussion. In the end the decision if to merge a change or not is made by the maintainers through consensus decisions. To most of the maintainers feature additions do not have a high priority because we consider other issues much more pressing. But at the moment - as already said - this PR does not wait for maintainer consensus, it waits for additional reviews and for changes requested to be made - and also in a way for consensus among mappers what highway=busway is to be used for (and what not). Regarding the two variants you created - both of them create a special case for highway=busway in interpretation of access tags - which would be confusing for mappers. The case to look at for reference (as pointed out in #4456 (comment)) would be highway=pedestrian. And by the way: It is self evident that someone submitting a PR thinks that this PR is a beneficial change to the project. If not they would and should not submit a PR. It is the function of the review process to discuss and determine if that is actually the case and to identify and solve issues with the proposed change. Hence it is fairly non-helpful to just reiterate the belief that the the PR is of benefit because it is dismissive of those who have reviewed the changes and those who might still want to do so. |
I think this commit addressed that problem:
|
Evidently not - like i pointed out this also creates a special case for highway=busway in interpretation of access tags. Our interpretation of the access tags currently is as follows: For most road classes if a road is tagged access=no or access=private it is rendered with a gray dashing centerline, if it is tagged access=destination it is rendered with a gray dotting. For highway=pedestrian this is not rendered because interpreting access=no/private/destination alone rarely makes sense for those. We have discussions about more elaborate interpretation of access tags (#214, #765, #3033) or removing access restrictions rendering on roads in general (#4137) but we definitely do not want partial changes in that direction as a footnote to a feature addition. |
That commit makes |
I think i have done all i can to explain myself - maybe someone else who understands me can explain it is a way that is better understandable. |
I think (but correct me if I'm wrong) @jdhoek and @imagico are not on the same plane when talking about access rendering. I think @imagico explained that rendering highway=busway with any form of non-access marking (grey dashes) and then rendering it only as a slightly darker, more blue highway=service when access=yes (or access is anything other than no or null) is completely opposite to how all other roads are rendered wrt access, which is a fair point. I think the resolution is to agree that highway=busway can be rendered without any of the regular non-access features. Everyone seems to agree with this, on the basis that a rendering is better than having no rendering, even if it is not semantically perfect. Off course, navigation programs still have to abide the access=no, but that is entirely separate from the rendering. I would like to add that I support the view that having highway=busways rendered would be beneficial as they are clearly a very different form of road than highway=service - which is not just my opinion, but that of all that approved the scheme. |
Dropping access markings is possible of course (just ask). Cherry-pick jdhoek@5c67381 on top of this PR to drop them: It works, sort of; in the sense that any rendering is better than no rendering. With And of course the Keeping the access markings seems more consistent to me. |
If you're in need of more opinions before merging, I think highway=busway should render as access=no by default. However, as others have mentioned, I would prefer any rendering over no rendering, so I'd be happy either way! |
Dear @pnorman TLDR: busways are currently invisible |
I personaly think that About the dash color, visually, my preference would be a purpelish dash rather than the grey one. This because it is a distinct road type for a specific purpose (providing a road for busses only). That is why I think a distinct dash color is allowed. This makes it also visually more pleasing. However, if the majority agrees to use a grey dash, that is also fine to me. |
Can it please be merged? Please? 6 months went by with the code ready and we are still with no rendering for busways. Release early, release often. I understand it is not very frequent in European cities, but it is very much used all around the world. The number of tagged ways would be much bigger if there was some rendering, even if not perfect. I can only speak for myself of course, but I am not going to migrate BRT ways in Rio de Janeiro, Brazil area from highway=service+access=no+bus=yes to highway=busway until it gets rendered, even if not perfectly, as the removal of from rendering is (IMHO) vandalism towards the maps users. If it just disappears other mappers would readd it with the old tags until it is rendered. |
The darker/grayer shown here makes it quite clear to me that this is a special type of road and I may not be allowed to use it. Dashes are clearer to render the implied access=no, but I think that when OSM Carto doesn't want special use cases per road type for access rendering, no dashes would be better. I also agree with other people here that any rendering is better than no rendering. Busways now show up as empty areas and every few days there pops up a new note on osm.org telling "The busway is missing", where the busway has been mapped correctly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m not convinced about the dark, bright purple casing color. This makes them look a bit like another class of highway in the motorway->trunk->primary sequency, which are currently magenta -> red -> orange. It would be better to use an unsaturated casing, like service and residential roads, I think.
On the other hand the current fill is quite light, it could be a bit darker and perhaps more saturated, to better contrast with living streets and pedestrian roads.
Have you tested the construction=busway rendering? It looks like the contrast between the fill and white parts may be low, though my previous suggestion may fix that. Perhaps try #ded4ef
for busway-fill
I agree with @imagico that it would be good to re-consider the rendering of highway=bus_guideway
in concert with this. We could unify the two renderings, since to bus riders it makes little difference if the bus has a guideway or a simple concrete road, as long as it is exclusively dedicated to transit buses.
Would you consider making that change? See #4567
Also, I again note that the current PR implement changes to openstreetmap-carto.lua to have z order, but that means that it cannot be merged until a database reload is planned.
@@ -1282,6 +1315,27 @@ tertiary is rendered from z10 and is not included in osm_planet_roads. */ | |||
} | |||
} | |||
|
|||
[construction = 'busway'][zoom >= 15] { | |||
line-color: @busway-fill; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems too low contrast vs white. You might need to try:
line-color: @minor-construction;
b/line-color: @living-street-fill;
But there are only 46 contruction=busway segments currently; maybe a dedicated rendering is not needed. https://taginfo.openstreetmap.org/tags/construction=busway
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But there are only 46 contruction=busway segments currently; maybe a dedicated rendering is not needed.
That would introduce a weird exception where none is needed.
Re: "Dropping access markings is possible of course” I recommend removing the default access markings, as well |
See the first post. |
We need to add this, but as soon as the change is adopted it will mean that
the layering order will change for newly added ways and recently updated
objects. The rendering database is updated each time a new or changed
object is added. But objects already imported into the database would not
change until the whole database is reloaded.
We do not want mapper to be confused by different rendering between new
highway=busway ways and already mapped ones, so we should not make the
change until we are ready to ask users to reload their rendering databases.
This suggests that we should also make changes to highway=bus_guideway at
the same time
…On Sun, Jun 5, 2022 at 11:33 PM Jeroen Hoek ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In openstreetmap-carto.lua
<#4456 (comment)>
:
> @@ -203,6 +203,7 @@ local roads_info = {
primary_link = {z = 220, roads = true},
secondary_link = {z = 210, roads = true},
tertiary_link = {z = 200, roads = false},
+ busway = {z = 180, roads = false},
Is there a technical reason not to add this? Because I understand that for
it to have effect the database reload is needed, but is its presence
harmful if merged before that is done? (Which is what you are implying.)
—
Reply to this email directly, view it on GitHub
<#4456 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKGGZBDRPF4EE4MERWM5M73VNWLUZANCNFSM5DHDO5IQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
highway=busway and highway=busway access=no Should render the same
I agree. But as with highway=pedestrian there should be not access dash
marking, because the tag is not needed and we should not imply that it is
present.
Instead, the new rendering should visually suggest that the road is not
part of the usual public street network, without requiring the use of the
dashes.
|
You placed your comment at a line in Thanks for giving concrete suggestions for the colours; I'll experiment with them. @jeisenbe By the way, replying via email causes your comments to show up here in the main thread, If you want to reply to specific comments on the changeset, it is better to use the web interface to keep those threads in one place. |
@pnorman rendering this will require a database reload to fix the z-value in openstreetmap-carto.lua. |
82581cf
to
e466368
Compare
Fixes gravitystorm#4226. Introduces rendering for `highway=busway`, an approved tag with over 11,000 uses. This fixes gaps being left on the map where bus-only roads are present and mapped. Fixes gravitystorm#3396, gravitystorm#3581, and gravitystorm#4567. Makes `highway=bus_guideway` match rendering of `highway=busway` due to an increasingly small distinction between the two in reality. Moves `highway=bus_guideway` to road layer. Includes rendering for unpaved busway and bus_guideway.
82b9c63
to
0156bfe
Compare
This is embarrassing for the OSM community, that it takes years to make such a simple tag have any render on the map. At least, make an interim render, for example matching the style of |
Render highway=busway, alter highway=bus_guideway
Fixes #4226. Introduces rendering for
highway=busway
, an approved tag with over 5,000 uses. This fixes gaps being left on the map where bus-only roads are present and mapped.Fixes #3396, #3581, and #4567. Makes
highway=bus_guideway
match rendering ofhighway=busway
due to an increasingly small distinction between the two in reality. Moveshighway=bus_guideway
to road layer.highway=busway
.)highway=bus_guideway
.)Before
(
highway=busway
is not drawn.)After
This is a variant of this PR where the rendering of
highway=busway
andhighway=bus_guideway
is combined; the latter differing only by a slightly more pronounced casing. There are two hue variants showcased here.This is a solution takes the dashes from
highway=bus_guideway
to help mark these roads as exclusively for public transport use. Another possible variant is to drop the dashes, and work with colours only. Having a compromise between the current rendering ofhighway=bus_guideway
and this PR is nice though.Plan Purple
Possible downsides: hue is close to
highway=motorway
(though not in this lightness).Plan Teal
Possible downsides: hue is used for leisure and nature as well (though not in this lightness).