-
Notifications
You must be signed in to change notification settings - Fork 819
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
Variable width rendering of aeroway=runway and aeroway=taxiway at high zooms #1853
Conversation
Width tagging is mentioned on http://wiki.openstreetmap.org/wiki/Tag:aeroway%3Drunway but not on http://wiki.openstreetmap.org/wiki/Tag:aeroway%3Dtaxiway Also, according to wiki mapping runways as areas is OK (both http://wiki.openstreetmap.org/wiki/Tag:aeroway%3Drunway and http://wiki.openstreetmap.org/wiki/Aeroways). Taxiway page on the other hand has "Mapping aeroway=taxiway as areas is disputed. You can join the discussion." and http://wiki.openstreetmap.org/wiki/Aeroways has "drawing taxiways as areas is disputed." In that case I have no idea whatever wiki or PR should be changed and is there consensus or controversy about how these features should be tagged. I think that it would be better to have consistent wiki and rendering (but I have no idea what and how should be changed). |
way, | ||
0.5*(St_Distance(ST_StartPoint(way), ST_EndPoint(way))/St_Distance_Sphere(ST_Transform(ST_StartPoint(way), 4326), ST_Transform(ST_EndPoint(way), 4326)))* | ||
CASE | ||
WHEN width ~ '^-?\d{1,4}(\.\d+)?$' THEN width::NUMERIC |
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.
use a float, we don't need arbitrary precision
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 is also allowing negative width and width up to 9999, neither of which seems to make sense
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 was just taken from how ele
tags are treated - you are right about the sign 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.
This entire section isn't the clearest, can you rewrite the logic?
Right now it's 0.5 * ST_Distance/St_Distance_Sphere * (width or St_Distance_Sphere/50 bounded between 12 and 75)
It might be better to do
0.5 * cos(ST_y(st_transform(st_centroid(way),4326))) * -- Correct for Mercator projection
CASE
WHEN width... THEN width::float
ELSE LEAST(GREATEST(ST_Length(way::geography)/50,12.0),75.0)
END
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 considered that but i don't see the gain (except for slightly shortening the code) and it would only work on web mercator. I suppose computationally ST_Length(way::geography) is identical to ST_Length_Spheroid().
Ideally mapnik should make available the tile center scale factor in a similar way as !pixel_width!.
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.
slightly shortening the code
Shortening the code and clarity is the reason.
only work on web mercator
Do you mean a web mercator database or rendering?
Ideally mapnik should make available the tile center scale factor in a similar way as !pixel_width!
Well, the scale is the same everywhere - the data is in mercator meters, the projection is mercator, and it's the same mercator meters / px anywhere in an image. All measurements are in some projection
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.
Do you mean a web mercator database or rendering?
Thinking about it, this may not be possible for different rendering projections. If it is possible, it's going to involve !pixel_width!
and height, as that's the only way we could get any projection-dependent information into the query.
The problem is that to get width
into rendered projection units, it needs to be multiplied by something for web mercator, and not for local projections where projection units are in meters and are close to true distance. Without a way to get the width into the units of the projection, we're stuck. If we could, we could turn the width into pixels with sqrt(!pixel_width!*!pixel_height!)
.
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.
Do you mean a web mercator database or rendering?
Both - although the only practically relevant case for the projection in the database differing from rendering would be EPSG:4326 in the database.
Currently AFAIK there is one case in the code assuming a metric coordinate system in the database:
https://github.com/gravitystorm/openstreetmap-carto/blob/master/project.yaml#L726
Practically this is only a problem for EPSG:4326 databases - all other coordinate systems are generally nominally in meters. Most relevant cases will be traverse mercator projections for local maps and polar projections like here. These would work right now but not with this change in the version you propose.
Well, the scale is the same everywhere - the data is in mercator meters, the projection is mercator, and it's the same mercator meters / px anywhere in an image. All measurements are in some projection
Scale factor refers to the ratio between projection units and ground units, exactly what you compute with the cos() term. There is no generic postgis function for computing it so you have to do it by hand. This differs for all conformal projections across the map area and is useful to know but usually is not really required per feature so computing it once per metatile would be sufficient.
I agree the wiki is not very clear here.
On a whole my primary motivation for this change is to improve what the style communicates to mappers regarding how to map airport features and to improve rendering quality across latitudes. Right now it essentially communicates that the mappers should map aeroways as lines for the low zooms and as areas for the high zooms by deliberately rendering lines in a fairly ugly way at the higher zoom levels. This is neither what is desirable from a data user perspective nor what is efficient to map nor what is documented in the wiki. |
In cases like this it is also OK to revert and start a discussion ( https://en.wikipedia.org/wiki/Wikipedia:BOLD,_revert,_discuss_cycle usually works ). Overall I strongly suggest to change wiki if it is wrong. |
Changed SQL as per suggestions by @pnorman. I realized that although this would constitute the first real dependency on web mercator projection this style is implicitly already strictly limited to mercator since styling is directly tied to zoom levels and the zoom level to scale relation is strictly specific to the projection. I would love to see this change (i.e. make styling normally depend on !scale_denominator! rather than zoom level) but this is a larger and separate project. I kept the more generic formula as comment for reference. BTW why the heck does postgis not offer a cosh() function? |
I guess the ultimate solution - not tied to any projection - is drawing it as area (with area=yes or probably with area:aeroway=*, if we need more complexity than that, as it would be reasonable analogy for already spreading area:highway=*). |
Computing the width does not work for all values with a unit. Would adding "m" be a tagging error? At least josm complains. |
Note there are currently 236 runways with a width value including 'ft' 102 including 'm' - this is negligible compared to the total number of width values tagged for runways. Cases with such values would fail gracefully - they would just use the estimation formula instead of the tagged width. Please keep tagging discussion out of here - i explained why mapping a runway as an area is fairly pointless from the rendering perspective. |
Sorry, I have not read your full description of this PR! However I think the need for the projection-specific solutions may be (or may not be) the reason to reconsider it one day - for now I feel hacks like this are in fact less elegant, because they are less universal. |
There are no projection specific hacks needed, you just have to draw the lines in their real width. This is no rocket science, the technical difficulties only arise from the limitations of the software used (postgis and mapnik) not supporting this natively. Telling mappers to spent time painstakingly drawing outlines without any factual gain and later spending even more time maintaining this data just to spare the style and software developers the need to deal with some projection math would be the real hack IMO. |
I'm not into this subject enough to judge myself, but your explanation sounds reasonable to me. |
sent from a phone
Actually when this was changed in 2012 the editor added a hint that by that time, the mapfeatures page already allowed for mapping runways as areas (and this is still so today: http://wiki.openstreetmap.org/wiki/Map_Features#Aeroway In some cases (unpaved airfields without markings), you can't survey other than an area, sometimes not even rectangular. |
sent from a phone
IMHO it's correct tagging, but not very common (I don't do it myself either) |
sent from a phone
having to guess a best fit center line seems more complicated and error prone to me, e.g. here https://www.google.it/maps/place/00052+Furbara+RM/@41.9949863,12.0141113,18z/data=!3m1!1e3!4m2!3m1!1s0x1328ac78d2712b3d:0x7f4f6743d1b68dca?hl=de-it |
In area:highway=* scheme we try to have best of both worlds: currently we draw the highway areas by hand, because these shapes can be really convoluted (especially on junctions), but it's expected that for the long segments of regular highways we would extract width from the tagged line. I guess speaking of aeroways automatic width rendering (as proposed in this PR) could be sufficient in most of the cases probably, and area definition would take care of the rest. |
I'm not sure about removing area rendering. |
END, | ||
'endcap=flat join=round' | ||
), | ||
highway, railway, aeroway, z_order, ST_Length(way)*ST_Length(way) AS way_area FROM planet_osm_line |
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 should be split onto multiple lines. Also
ST_Length(way)*ST_Length(way)
is not the areahighway
andrailway
should probably both be NULL, unless you want to handle combinations as areas
If anyone could show a case where something is correctly mapped as a runway area that can not be equally represented by a simple linear way with width tag that would go a long way towards convincing me. From what i saw runway areas are currently either hand drawn versions of the buffering i try to do here or abuses of the runway tag to map taxi areas or whole airfields. |
Runway and taxiway should be threated like most of highway=, allowing only the linear represenation. Area tagging should be done by area:aeroway=. |
sent from a phone
did you have a look at my example (Furbara)? the runway doesn't look to be a perfect rectangle. |
Yes, from the imagery (see here) i cannot reliably identify a runway here. There is some structure visible on the grass but it is unclear to what extent this is a mowing pattern or an actual runway delineation. Barring a locally clearly visible outline i would say this is equally non-verifiable to map with a runway as a linear way and as a polygon. In general this does not look unlike a typical glider airfield which is often not more than a large, flat and maintained grass area where position and direction of the winch and landing area often varies depending on wind directions. Tagging the whole grass area as aeroway=runway seems inappropriate here in any case. |
2015-09-21 18:47 GMT+02:00 Christoph Hormann notifications@github.com:
Actually this is a military airport and is unlike the glider airfields you |
OK - i put in the new approach suggested by @pnorman. Some thorough review is probably in order since this is somewhat complicated. One small problem occured with the bounding box expansion since tilemill seems to always call the query with some strange large bounding box that causes a postgis error. So i added an additional check for that. |
and only once, possibly corresponding to an initial check? I recall something similar happening once and ended up doing st_intersection(bounds of the world, something calculated from !bbox!), and wasn't very happy with it. |
I don't see that tagging runways as an area is inherently a bad idea. It's easy for mappers to do (I have no idea how many metres wide 09R is at LHR, but I could draw an outline in iD or JOSM). It fits with the mapping of similar-scaled features. It's common practice among mappers. And it certainly makes the use of the data easier for polygon fills, as per the (somewhat complex) SQL discussions in this thread already. So I'm happy to see the width tag used to improve the rendering for when areas are missing, but I think throwing out the area rendering is likely going to far.
I'd suggest caution about over-analysing the current behaviour. It's not clear that there was any degree of deliberation about the particular widths used, and whether they are deliberately wider than real width or not. For zooms where the lines are currently wider than would be expected, I would suggest the airport icon is more important for legibility than an extra few pixels. |
I did not mean to - i just meant to point out the current low zoom rendering is consistent and meaningful but less so at high zooms and this change hopefully improves the latter.
I understand that but consider this primarily a limitation of the current editors which do not visualize the width tag or allow interactive manipulation of the width value. Note while this might seem a very specific subject and not really worth an elaborate argument it touches a fairly fundamental question regarding the data model, namely if geometry and tags are strictly separate and geometry information should be modeled using the geometric elements currently available (nodes, ways and areas) or if tags can define aspects of the geometry of features in OSM. This does not only affect dimensional tags like (sorry for straying off-topic - this is certainly not something that should be discussed here but i wanted to point out the broader context of this question).
Currently this PR renders the line with real width regardless of the presence of a polygon representation of the same runway. While it would be possible to detect this and not render the line when a polygon exists i do not consider this desirable because it would communicate the polygon mapping has preference over the line mapping with width tag. Always rendering both when present would of course be possible (as for taxiway) but this might be confusing. |
Probably that will work. |
FWIW: The widest and longest runway might be at https://en.wikipedia.org/wiki/Edwards_Air_Force_Base
The runway seems to be missing in OSM, it's hardly visible anymore on the aerial images: |
Rebased and added new bounding box test. I did not change the general principle since there has been no comment regarding #1853 (comment) Regarding Edwards Base runways - the larger ones on the dry lake are all marked as separate parallel runways individually no more than a hundred meters wide. So they should be fine with this change (as long as correctly tagged). |
Still waiting for opinions regarding area rendering - those who voiced concerns about this: please comment on the options outlined in #1853 (comment). |
Sorry, I haven't had time to look at thjs (reading a discussion of this length does take quite some time). |
I think we should try to keep the complexity of the stylesheet down. Therefore I'm not in favour of variable width rendering. It adds relatively a lot of complexity for a small gain. Perhaps something that supports this in Mapnik would be useful? In any case I'm going to close this PR. Thank you for the effort though @imagico! And sorry for long time to review. |
To avoid unnecessary work in the future: I suppose this means that ground unit rendering of non-explicit geometries is generally not considered desirable and #1290 is to be closed as wontfix - because any such thing would mean a similar level of code complexity. Regarding the vague hope for future native support in Mapnik see also #1975 (comment) Anyway thanks for considering and thanks to @pnorman for the help with the code details. |
Thanks for pointing out, I closed the issue too. Sorry, sometimes tough choises need to be made! |
I can accept the choice although i strongly disagree with it. This poses a hard limit on improvements of the rendering quality and makes a truly latitude-agnostic rendering impossible. #1126 would then probably need to be closed as well - requires the same techniques although in a slightly different context. |
I think latitude issues ultimately need to be solved by rendering data in local projections that minimize distortions. This is one of the things that will be a breeze with my ArcGIS Renderer, as ArcGIS handles on-the-fly re-projection of data really nicely. In fact, I started out rendering data in the "default" "Web Mercator" projection to compare the rendering results with carto, but have switched to local country specific projections for test renderings for its nicer more consistent and less distorted look of the rendered maps. |
This implements what i suggested in #1290 for aeroway=runway and aeroway=taxiway.
Reasoning as outlined there is that current rendering of these features is using line widths larger than real width at lower zooms (which makes sense for map readability) and line widths smaller than in reality at the high zooms (which does not make much sense). This change keeps the current line width at the low zooms but renders these ways in real width in addition to ensure they are never drawn thinner than in reality.
Width is taken from the width tag, if width is not tagged it is estimated for runways based on the runway length. For taxiways the default width used is 6 meters which is much too low for any but the smallest airfields - making a better estimate based on nearby runway length for example would be possible but might be confusing for the mapper.
Previous rendering of runways mapped as polygons has been removed since a runway is always perfectly described by a two node way and a width so there is no good reason to map a runway with a polygon. Taxiways continue to be rendered based on mapped polygon data as well (any complex geometry structure is technically a taxiway and not a runway).
The change requires the EPSG:4326 SRS to be available in the database (which IIRC is currently not required). OTOH it should be generic and also work in other projections than web mercator. I think this is the most elegant way to do this with the current constraints but if there is a better way i would be open to suggestions.
Examples from Frankfurt airport:
z15 before:
z15 after:
z16 before:
z16 after:
z17 before:
z17 after:
Width tagging of runways and taxiways is currently fairly incomplete and sometimes with too large values (probably indicating clearance instead of physical width on the ground). But this will probably be sorted out quickly once it is rendered in the map.
The zoom level where this change has a visible effect depends on latitude, the higher the latitude the earlier it happens, here for Tromsø -with no width tag so width is estimated:
z15 before:
z15 after: