-
Notifications
You must be signed in to change notification settings - Fork 824
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 name labels of bays and straits from z14 only, lakes from z5 only #3750
Conversation
I support this but it is clear that we need to establish consensus on the matter before this can be merged. As a point for discussion: A theoretical alternative to removing the way_area based labeling decisions would be to actually render the geometry by drawing things in a different color than the ocean. This would provide feedback on the geometry and thereby address some of the arguments against using way_area. I would be against this since (a) it is bad cartography even with accurate and verifiable mapping and (b) it would still render bays and straits fundamentally different depending on how they are mapped which is not desirable IMO. I like to clarify one thing regarding:
I have not made any promises that i will actually write code for that. What i have said is that this is possible to do, most experienced developers here would be able to do this (put the coastline data and the bay/strait features into a temporary PostGIS or spatialite database and do some processing). Anyone who wants to work on something in that direction is welcome to ask for advise on specific practical questions. But independent of the question of implementation we would need to decide on if we want to actually have something like that in this style. This we would need to discuss based on a specific proposal that can be tested. I think this would probably be a good test case for trying the fundamental idea and evaluating the practical effects of it but I would not be in favor of connecting a decision on this PR with a definite forward promise to make this decision in a certain way in the future. |
Sorry, I'm not sure why I thought there was a plan to develop the script. Wishful thinking perhaps? I've edited the original comment. Should I open a specific issue to discuss preprocessing the bay, sea and strait nodes in this way? |
👍 - does not change current rendering, but helps to prevent vandalism in a sane way, so I'm OK with that
👍👎 - I'm OK with rendering them later, as they typically have less land borders, but since they can be quite big, I vote for z7+ to avoid looking for labels on the water (this way even biggest ones will be still detectable with the land still in sight)
👎 - I'm against both dropping area rendering and way_area (at least for now) In short: the main reason is there are more problems with big (especially global) objects and with water objects in OSM. The discussion on tagging is currently open, but is far from being resolved, so I believe the most responsible action is not to decide now how to change rendering and wait for the outcome. A bit longer: these problems are really general and verifiability of nodes or lines is questionable as well. For small objects it is not that painful, but for global objects local knowledge does not really apply - the area shape can be fuzzy, the line can have unreliable ends and shape and position of node can differ by many kilometers. Since their size may vary, I would not remove rendering for dots and oppose rendering ways (if they are small enough we can reasonably round the area down to the line or node), but for larger ones using nodes is abusing the simplification - this way or another you start with some shape to find the middle of the object (at least to know you really put it inside), so it's better to show it instead of hiding the problem (this is exactly making it less verifiable IMO). There could be different solutions to this - maybe global objects would be banned from OSM (but then geometry doesn't matter anyway), maybe there will be some special rules to cover such special cases - who knows? Maybe we would define bays and straits in a different way, so there would be more clear how to tag them. Yet it's too early to know and act, it needs some time to settle and let people decide with both discussion and practice. |
@kocio-pl - for understanding your position: Would you be fine with keeping the way_area logic but introducing in addition a cap of way_area at 270000 (i.e. any polygon larger than this is interpreted to have only this way_area)? That would remove the incentive to mappers to draw labeling polygons for straits and bays to make them show up at the zoom level they desire since bays/straits mapped with nodes or linear ways are not shown later than ones mapped with large polygons then. |
I'm not sure if I understand you properly, so I'll try explain this way: I'd like to keep the way_area logic to retain a sense of size (since this gives better feedback than removing it), but the initial zoom level is moved so they start showing later when zooming in. |
Which is exactly what a cap on the raw way_area value would do. A way_area of 270000 is approximately 3000 way_pixels at z14. |
Place=island text labels are first rendered at z4. None of the other text labels in this layer are rendered earlier now
The thing is, the way_area logic currently only works at zoom levels below z14. Small bays currently all render at z14 no matter how small they are. That's why I just removed the Can anyone confirm if the |
Current way_area logic for bays and straits can be found in: Lines 342 to 368 in b117a29
That means the zoom level threshold is disabled at z17+ but the zoom level logic for the label size is the same at all zoom levels. |
That's what I thought, but when I did test renderings of Waikoka pond (the smallest "bay" or probably an estuary in Hawaii), it still rendered on z14. (I think you are correct about straits). Here's a screenshot from openstreetmap.org: z18 with the closed way selected: I believe these lines in the openstreetmap-carto/amenity-points.mss Lines 2524 to 2538 in b117a29
|
I added another commit which changes the |
Ah, yes, there we have some chaos in the code that needs to be cleaned up - there is also Lines 372 to 389 in b117a29
So all together three different variants in two different files with rules that overlap in application. All the more reason to remove the way_area logic - since it is not working consistently at the moment anyway. 😉 |
Ah, who added that! How careless. Let me check git blame ... Oh, I did it. Oops. So the current initial zoom level for both straits and bays mapped as polygons is:
That is, currently they are rendered as early as z0 if they are mapped as polygons that take up more than approximately 3000 square pixels on the screen. But at z14 and higher all bays and straits are rendered no matter what size. Nodes are also rendered at z14 and above. (Straits mapped as linear ways are rendered at z14 and above, but bays mapped as linear ways are not yet rendered; I'll fix this soon) |
@kocio-pl said "I vote for z7+ to avoid looking for labels on the water ... I'm against both dropping area rendering and way_area". The PR does not propose stopping rendering straits or bays mapped as areas (that is, those mapped as closed ways or multipolygon relations and imported as polygons). As far as rendering, there is not any area fill or outline rendering for bays and straits currently. Do you mean you think it should still be something like this for natural=strait and natural=bay?
The problem with this code is that it has been pushing mappers to change the mapping of large bays, straits and seas. As of March 2018, 1 year ago, the majority of all seas, bays and straits were mapped as nodes. But in the past year the number of natural=bay relations has increased from 2,000 to almost 5,000, while the number of natural=bay nodes only increased from 45k to 46k. Relations tagged natural=strait increased from 290 in March 2018 to 945 now in April 2019, while nodes increased only slightly. In early 2018 there were over 120 place=sea mapped as nodes and less than 20 mapped as relations. Now there are 68 place=sea nodes and 63 place=sea relations: the total number of mapped seas has not changed, but nodes are being replaced with polygons and being double-tagged with "natural=bay". Notice how stable the tagging was for many years until the past 6 months: We shouldn't be discouraging mappers from using nodes or ways for large bays and straits by only rendering the labels for those mapped as areas at low zoom levels. |
Sorry for editing your posts - I've just added missing "`" in both because formatting was broken and I had hard time reading it. The problem with nodes is that they lack information about size, which can vary a lot, so their rendering is trimmed to the lowest common size and I have found no way to improve it currently. That is the cost of avoiding area geometry for describing areas - some additional information is needed anyway (for example for cities we use population data to rank them). For small bays it works, but for big ones it doesn't. If there was tagging type which can differentiate small bays from big ones, it could be easier even on nodes, but I'm not aware of this. |
Thanks! Sorry about that
The coastline is already there, and we can compute the distance from the node to the coast to find out approximately how to render the strait or bay (or sea). Above: "this is possible to do, most experienced developers here would be able to do this (put the coastline data and the bay/strait features into a temporary PostGIS or spatialite database and do some processing). " #3750 (comment) The alternative is that we could add a darker outline for bays and straits so that mappers would know when they were incorrectly drawn as a rough sketch instead of along the coastline. But I don't think this would work for seas, and it will look not so great. I'm quite concerned that seas are being double-tagged as bays and redrawn as polygons, eg: the Bothnian Sea has been mapped as a node for over 7 years as place=sea - but this doesn't render yet: https://www.openstreetmap.org/node/1195287427 But now there is a "Gulf of Bothnia" relation made of just 2 ways roughly drawn 5 months ago: https://www.openstreetmap.org/relation/9048256 |
Note the idea of avoiding area geometry for describing areas is somewhat misleading here. A polygon (closed way or multipolygon relation in OSM) is just a way of defining a two dimensional entity (an area) through an explicit representation of its outline. It is not that mapping a bay or strait with a node or linear way typically lacks significant information about the feature in question, that information is already available in form of the coastline geometry. It is just that in our rendering toolchain that information is not readily available for making rendering decisions. As @jeisenbe explained with #3438 and #3144 we have created an enormous incentive for creating polygons to represent bays and straits but provide no feedback on the geometry of these polygons. This has led mappers to create thousands of polygons, many of which they would not have created without this incentive. This is something we should not do - no matter how convenient it is for us within the technological framework we have chosen. Note it is not that for small bays there is any less need for decent importance rating and label placement than for large bays. In a zoomable map this is a completely scale independent problem. It is just that at higher zoom levels the problem is more hidden and can be ignored more easily than at low zoom levels. If we can't achieve consensus on not using way_area any more for labeling decisions for bays and straits at z<14 i would - as already indicated - propose to explicitly display the geometry of bay and strait polygons for providing proper mapper feedback on these geometries. I don't really like that but it would be much better than the status quo. It might be a solution to the equal dissatisfaction of everyone. |
Sorry for not being too verbose - I still have not too much time and energy. You both pointed out a lot of interesting problems I'd like to talk about or ask about details, but it looks like this PR is a "fork bomb". 😉 So I would like to propose:
Anything that could make this problem less complex would also help, do you have some ideas? Some background (I doubt it will close the discussion, but I believe it can at least clarify a bit my views):
|
@kocio-pl - i try to understand the long term strategy behind your suggestions. You seem to insist on maintaining making the decision at which zoom level to start labeling a bay or strait mapped with a polygon from way_area - which is the central idea of this PR to abolish. If that is not the case please say so - consensus hinges on this here so it is important not to have misunderstandings on the matter. But apart from that you indicate to support improving the labeling of bays/straits mapped with nodes - which based on the discussions about this we had so far would mean pre-calculating an importance rating for them using the coastline data to select the starting zoom level based on that. What i am not sure about is this: If such a method is developed and if we achieve consensus to use it in this style if you'd want to use it for nodes only, that would mean still keeping the way_area logic for polygons while rendering nodes with a completely different system in parallel or if you then would be fine with abolishing the way_area logic and labeling both nodes and polygons using the same pre-calculated rating for selecting the starting zoom level. |
I've updated this branch to fix a merge conflict. It would be good to have input from the other maintainers. I see 3 possible long-term solutions:
|
@imagico My long term strategy is to balance the gains/problems with each geometry choice, since I see no perfect solution for this problem. I find using way_area for water objects pretty natural. If we find a way to prioritize water objects, I would use it to make sure that parts are never rendered before the whole objects, so it's rather about initial zoom level, not taking away the sense of object's scale.
|
I don't understand this comment. EDITED: Oh, I think I see my mistake. I changed the text above to clarify: the rendering of name labels for areas should not depend on the With this PR, bays and straits would be rendered the same whether they were mapped as nodes or areas: they would get a name label at z14, on the center of the polygon or node. Currently there is a special rendering for bays and straits that are mapped as polygons which allows them to be rendered at lower zoom levels and with larger text.
This isn't necessary to show the geometry, because the name label is centered exactly over the node, so if you zoom in to z19 it shows the location of the node quite precisely. This is the same situation with place=village/hamlet: we just render the name.
This PR would not favor bays or straits that are mapped as nodes. They would get an identical rendering to those mapped as areas. Currently it is those that are mapped as areas that are favored by the special rendering. I'm mentioning seas because they are quite similar to bays, and some seas (Baffin Bay, the Bay of Bengal) are even named "X Bay" in English, or "X Gulf" (eg. Gulf of Mexico). If we render bays as soon as z5 when mapped as an area, then we also need to render seas in this way. It's rather inconsistent to show the Bay of Bengal but not the equally large Arabian Sea on the other side of India. |
@kocio-pl - i still don't understand your position. You write I'm all for improving rendering nodes and i am trying to understand what you mean by that. Does this mean you are only for improving rendering of nodes but insist on keeping the way_area logic (and thereby direct mapper control over the labeling) for polygons? This is a pretty fundamental question regarding this matter. 👍 on 1. |
Yes, that's what I mean. |
Seas and oceans have two huge problems with regard to rendering, which make them different enough:
That's why I would keep them in separate tickets. |
OK, I guess I'll open a new PR to add the outlines and a separate PR
to render lakes from z5 only?
…On 6/17/19, kocio-pl ***@***.***> wrote:
Closed #3750.
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#3750 (comment)
|
Yes, please. In a matter of fact I consider such complex PRs to be a problem and prefer smaller ones, which are easier to test and apply. |
@kocio-pl - in issue #2068 it was clear that @imagico opposed PR #3144 (which rendered bay areas sooner) but the PR was merged without discussion only 1 day after it was opened. Perhaps we can reconsider whether a lack of consensus for the original changes in #3144 should mean that the change should be reverted, as in #3955 |
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 agree with these changes, but would like to see a rebase before giving an approving review.
@jeisenbe would you have a chance to rebase this PR? |
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 figured out how to resolve the conflicts, it wasn't too bad
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.
Checked that this works as intended.
As already indicated above in #3750 (comment) i support this.
This also resolves #4546.
The entire water names section of code could use cleanup. |
…tended by the PR
Fixes #3634 and #3705
Reverts #3438 and #3144
Related to discussion of
way_area
based name label rendering in #3284 and #2700, also see #2702Changes proposed in this pull request:
way_area
to set zoom level and text size for natural=bay and natural=strait text labelsExplanation
Since Rendering of strait from z10 #3438 and Adding rendering for big natural=bay #3144, natural=strait and natural=bay have been rendered identically to lakes (natural=water and landuse=reservoir) when mapped as an area (closed way or multipolygon relation). Depending on the
way_area
, they will render as soon as z0 in theory, and in practice there is currently a bay rendered on z3.This led to issue Limit natural labels rendering to z4+ #3634, which recommends that natural area labels should not be shown at z4 and below.
The mapping of bays and straits as areas is controversial, because the non-coastline borders of these features are not verifiable by mappers, and the coastline is already contained in the database. Also, mapping large bays or straits with multipolygons relations can lead to difficult to manage relations with many hundreds of members.
Because of this, some mappers have taken to mapping a bay or strait with a rough outline that does not follow the coastline. This is encouraged by the current rendering, because these features are only shown as a text label with the name, and there is no mapper feedback about the shape or borders of the area in the map rendering.
See also: https://www.openstreetmap.org/user/imagico/diary/43957 [Warning, dark humor]
Recently a text label rendering was added for straits mapped as linear ways in Add rendering for straits mapped as linear ways #3733
We recently stopped rendering tourism=attraction based on
way_area
because of this same problem, in PR Reduce priority of tourism=attraction and render from z17 #3603Previous to PR Restrict rendering of natural=cape to nodes, change font to standard #3732 natural=capes mapped as an area were rendered based on
way_area
, but this has now been removed.Bays, straits, and malls are the only 3 features that currently have text labels that are rendered based on
way_area
without an outline or area fill rendering.This PR will fix Limit natural labels rendering to z4+ #3634 and Do not render water areas names at z2, z3 #3705 by limiting natural=water, landuse=reservoir, landuse=basin and waterway=dock rendering to no sooner than z5. Even the largest lakes in the world do not need text labels before this zoom level, and natural=water is being abused to map marine water areas for label rendering.
The initial zoom level will be set to z14 for all straits mapped as nodes, ways or areas and all bays mapped as nodes and areas. This level has previously been tested and deemed an acceptable initial zoom level in PR Add rendering for straits mapped as linear ways #3733 .
(The PR author plans to submit another PR to add rendering for bays (specifically fjords) mapped as linear ways similar to #3732, and an additional PR to limit the rendering of malls to no sooner than z13 (where the retail landuse fill color is shown) and add a fill or outline in retail color, which should finish resolving the
way_area
based text labels issue.)While many of us would like to see bays and straits rendered at low zoom levels in this style (the author included), we have a responsibility to support good mapping by providing appropriate feedback. Therefore in the short term this PR is the best solution.
@imagico has suggested a script could be written that would calculate the correct initial zoom level based on a node and the position of the coastline. This would allow us to restore the rendering of bays and straits at low zoom levels, and not only for areas but also for nodes. It would also make it possible to render seas in a consistent fashion, which is not currently possible because these are mapped as nodes. However, there is not yet a volunteer to implement this. [EDITED]
Test renderings:
Yam Bay, Hawaii - mapped as an oval-shaped closed way rather than straight across from the ends of the bay; this causes it to render 1 zoom level sooner than otherwise:
z13 before
After
z14 before
after
z15 before
after
Waioka Bay, Hawaii
way_area
based filtering which should have prevented rendering until later. So the currentway_area
adjustment only shows labels sooner, never later.z13 unchanged
z14 Before
After
z16 Before
z16 After
Shelikof Strait, Alaska
z6 Before
after