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

The use of way_area #703

Closed
Rovastar opened this issue Jul 4, 2014 · 22 comments
Closed

The use of way_area #703

Rovastar opened this issue Jul 4, 2014 · 22 comments

Comments

@Rovastar
Copy link
Contributor

Rovastar commented Jul 4, 2014

I thought we could have a discussion in one place about the use of the way_area. It keep cropping up on many issues as a solution but until recently it is seldom used.

Many issues on the carto style can be improved if we use way_area to define if they are displayed or not at each zoom level.

e.g. we have

  [leisure = 'golf_course']::leisure,
  [leisure = 'nature_reserve']::leisure {
    [way_area >= 150000][zoom >= 14],
    [way_area >= 80000][zoom >= 15],
    [way_area >= 20000][zoom >= 16],
    [zoom >= 17] {

Now should we use this more. Personally I think we should it makes logical sense and will improve the map no end. It can be used in so many cases from displaying names for museums, lakes, recycling centres, etc to massive nature reverses being displayed at level 5.

Doing the catch all of x features should be rendered on y level as we do at the moment is not a best solution as bigger things are normally more important and should have more visibility.

The issues against this seem to be it will be computational heavy but I don't understand how much. Obviously there is a some.
And probably it would add more confusion/verboseness to the map code.

If we are to do this I propose having
a) consistent values
I think in gneral x size will work on y zoom level
e.g.

    [way_area >= 150000][zoom >= 14],
    [way_area >= 80000][zoom >= 15],
    [way_area >= 20000][zoom >= 16]

are these the most accurate and optimized areas for these zoom.
b) have them as variable (well constants) defined
e.g.
way_area_for_z14 = 150000, etc
and use these through the style.

c) using these throughout the carto style for existing features and new features.

Any thoughts welcomed.

@gravitystorm
Copy link
Owner

Yep, in general it's a good idea. Since the way_area is computed by osm2pgsql during import, there's much less performance penalty than there would be using st_area(way).

However, at the lowest zoom levels there will be an impact of sifting through all the ways looking for the ones with a sufficiently large way_area. So let's not go nuts with stuff appearing on zoom 5.

Also, there are problems with using this on adjacent areas. For example, large rivers are often split up into smaller polygons, and many small adjacent polygons might not be matched by an area filter yet the gap is noticeable. We can't force mappers to create large areas out of lots of adjacent small ones. We have similar issues where e.g. forests are split up by roads running through them.

We can do b), unfortunately, since cartocss doesn't support constants in filters, only in attribute values.

My final comment is that we will probably have multiple hierarchies - one for polygon fills where a handle of pixels is worth rendering, one for icons where it might make sense to only show an icon when the area covers a few hundred pixels, and one for labels where the feature might need to cover a reasonable proportion of the screen.

But to end on a positive note ( ;-) ), yes, it's a good idea.

@pnorman
Copy link
Collaborator

pnorman commented Jul 4, 2014

We can also move some of the way area checks into the SQL and use the pixel size parameters - this allows MSS like this

[way_area >= 150000][zoom >= 14],
[way_area >= 80000][zoom >= 15],
[way_area >= 20000][zoom >= 16]

to be condensed into one check, either by passing area in pixels out, or doing the filtering in SQL.

Thinking about it, I'd like to remove any explicit references to way_area from the style, but replace them by way_area/pixel size.

Filtering in the SQL has advantages for low zooms, but then means that we can't use the same layer definition for rendering the fill and for rendering the name.

@Rovastar
Copy link
Contributor Author

Rovastar commented Jul 4, 2014

@gravitystorm I agree with all that and understand (most of) those points but didn't want to get too much into it on an initial chat.
I know riverbanks, etc are split up.
I wasn't thinking of anything too radical for that case. Mostly water is fine. Expect for the case we had before (can find it now) where we had thousands tiny water areas showing up on zoom=7 and stuff. Which is a reverse case to the major of uses of way_area to what I talked about.

Wasn't planning on going nuts. That was referencing a case of huge country size nature reserve that came up before. I was just thinking we render a couple zoom checks either side of what we have have at the moment not going to 5 just like in the example.

My concerns were for @pnorman saying that CPU heavy. I can't see it so much as as you say we are not working it out on the fly.

Didn't realise that you could not use [way_area >= @ myconstant ][zoom >= 14], etc would have made life easier.

hierarchies yes, one for label placement one for fill. That is what I was thinking.

oh the replys are too quick now.....
@pnorman The only problem with moving it all to the SQL is it might not be good for all features.
Say you have "name": "landcover" you might often want different features processed differently for the way_area (or way_area/pixel_size (will that be even more CPU heavy though)). A one size fits all might not be best there.
For some cases sure but for many others it might not be useful.
And from a user/contributor point of view it is easier to work these than potentially changing the SQL.


Anyway good to see we have agreement thus far.
I suppose next is defining how (way_area vs way_area/pixel_size, SQL or stylesheet) and then when we do this. For this stage the arbitrary 2.x or 3.x one. Should new additions be following this, etc
What values we should use. way_area> xxxxxx for zoom x or way-area/pixel_size > yyy for all zooms, some zoom, etc.

But to end on a positive note ( ;-) )

;-) positive endings to osmcarto chats they are getting rarer!! :-)

@imagico
Copy link
Collaborator

imagico commented Jul 4, 2014

Normalizing way_area to pixel sizes seems a good idea, this would make use immensely more intuitive.

Of course filtering by polygon area can be and is also frequently abused - for labeling things it is usually fine but for selecting features to be displayed in general it will cause problems, especially when objects come in a broad distribution of sizes (as usual for natural features like lakes, islands etc.). Simply leaving out everything below a certain threshold can create an awkward bias then - a large number of small lakes/islands will be left out while a few larger ones will be shown even if their total area is much smaller. I don't want to point fingers but there quite a lot of maps out there where you can observe exactly this in a very unfavorable way. As @gravitystorm said there are also problems when arbitrary splits are used as well as in the opposite case of large but relatively 'narrow' polygons like long river sections.

@Rovastar
Copy link
Contributor Author

Rovastar commented Jul 4, 2014

There are no plans to remove these only when they cause problems.
like:
#73
Often these will be for labels.
The general feel of the OSM style for things nature features will remain unchanged.

@vincentdephily
Copy link

While matching a pixel size rather than a way area is tempting and would feel easyer to the style writer, I don't think it's such a good idea :

  • It doesn't make the style code much simpler, because in many cases you'll want to match the zoomlevel too.
  • The query will actually be slower, because there's more data in the db (one value per zoomlevel) and the query might be more complicated. I doubt it'll make much of a difference, but it's garanteed to be a negative one.

Also, we'd end up making different decisions at the equator and at the poles. To be honest, I'm not sure wether that's a pro or a con.

@pnorman
Copy link
Collaborator

pnorman commented Jul 8, 2014

  • It doesn't make the style code much simpler, because in many cases you'll want to match the zoomlevel too.

The cases above would simplify

  • The query will actually be slower, because there's more data in the db (one value per zoomlevel)

No there isn't. Technically dividing way_area by pixel size takes some time, but it's the time to perform a floating point division, which is not an issue.

Also, we'd end up making different decisions at the equator and at the poles.

It changes nothing in this respect - we have no square meters, it's square mercator meters.

@vincentdephily
Copy link

The query will actually be slower, because there's more data in the db (one value per zoomlevel)

No there isn't. Technically dividing way_area by pixel size takes some time, but it's the time to perform a floating point division, which is not an issue.

That's probably a misunderstanding on my part. I tought the area -> pixel conversion would be done at import time (which implies storing one value per zoom in the db, and therefore more IO), but you seem to imply it'll be done at query time (which has negligible performance cost but noteworthy query maintenance cost).

All in all I'm still not sure it is worth it... But it's a personal preference, and ye are looking at the code much more than I am.

@matthijsmelissen
Copy link
Collaborator

We can also move some of the way area checks into the SQL and use the pixel size parameters - this allows MSS like this

Hmm, I thought about that, but I'm not yet sure how to do that. If you have a concrete idea, could you point me in the right direction @pnorman?

It seems you cannot do too fancy things in zoom selectors. For example, something like [zoom >= 12+1] is already unvalid CartoCSS.

I think in the end we want to write something equivalent to [way_area >= 2.4*4^(22-zoom)], but I'm not sure how to express this. Note that the 2.4 here can be adapted to display smaller or larger objects.

@pnorman
Copy link
Collaborator

pnorman commented Aug 8, 2014

Hmm, I thought about that, but I'm not yet sure how to do that. If you have a concrete idea, could you point me in the right direction @pnorman?

do the math in the SQL

(SELECT 
    way, 
    way_area/(!pixel_width!*!pixel_height!) AS way_pixels 
  FROM ...) AS water

See the PostGIS Layer documentation.

@matthijsmelissen
Copy link
Collaborator

I see, thanks.

@Rovastar
Copy link
Contributor Author

Rovastar commented Aug 8, 2014

@pnorman I presume you can use way_pixels in the style script.
e.g. pseudo code
Zoom > 16 and way_pixels > x then do stuff

@pnorman
Copy link
Collaborator

pnorman commented Aug 8, 2014

@pnorman I presume you can use way_pixels in the style script.

Mapnik doesn't see the inside contents of the SQL, just the columns that result.

@matthijsmelissen
Copy link
Collaborator

Be careful when using way_area selectors: I discovered a tricky and apparently very random parsing bug in Carto, see mapbox/carto#369.

@pnorman
Copy link
Collaborator

pnorman commented Sep 11, 2014

Wow. That's oddly specific. As we'd be dealing with floating point selectors we could make sure that every area has a .0 at the end and dodge it, but I wonder what else lurks.

@matthijsmelissen
Copy link
Collaborator

we could make sure that every area has a .0 at the end

Nope, that doesn't prevent the bug.

@Rovastar
Copy link
Contributor Author

Is there any evidence that way_area is effected?
After all we use way_area and condition against it already with no problems.

@matthijsmelissen
Copy link
Collaborator

Yes, that's the way how I discovered the bug. It is very specific, and only occurs when way_area's are nested and the inequalities are exactly like in the example (as far as I found out).

@matthijsmelissen
Copy link
Collaborator

And another strange bug in Carto: mapbox/carto#370

matthijsmelissen added a commit to matthijsmelissen/openstreetmap-carto that referenced this issue Sep 12, 2014
This commit changes the rendering of landcover labels.

For the purpose of this commit, with 'landcover label' we mean text connected to
a background colour or pattern rendering, and not connected to an icon.

* All rendered landcover tags now have their name rendered (resolves gravitystorm#537). We
  add labels to the following tags:
  * natural=beach,scrub,grassland,heath,sand,desert (partially resolves gravitystorm#788)
  * highway=services,rest_area (resolves gravitystorm#575)
  * aeroway=apron
  * power=station,generator,substation,sub_station
  * tourism=zoo
  * military=barracks
* The minimum zoom level of labels is now defined based on the number of pixels
  rendered (resolves partially gravitystorm#703, resolves gravitystorm#861, resolves gravitystorm#913).
  Labels are rendered from 3000 pixels (but never earlier than the corresponding
  landuse is rendered).
* Font size now also depends on way_pixels. In other words, larger objects get
  a larger label, in three steps (resolves gravitystorm#308).
* Landuse labels are now rendered oblique, to easier visually tell them apart
  from village and POI labels.
* All labels are rendered in a colour similar to the landuse they belong to
  (using landuse color variables, resolves gravitystorm#56). Also some existing colours
  are changed, in order to make them clearly colourful but still readable.
* The text-halo-radius and text-wrap-width properties are made consistent across
  landuse types.
* Font-size, wrap-width an face-name are now defined by easy to change
  variables.
matthijsmelissen added a commit to matthijsmelissen/openstreetmap-carto that referenced this issue Sep 21, 2014
This commit changes the rendering of landcover labels.

For the purpose of this commit, with 'landcover label' we mean text connected to
a background colour or pattern rendering, and not connected to an icon.

* All rendered landcover tags now have their name rendered (resolves gravitystorm#537). We
  add labels to the following tags:
  * natural=beach,scrub,grassland,heath,sand,desert (partially resolves gravitystorm#788)
  * highway=services,rest_area (resolves gravitystorm#575)
  * aeroway=apron
  * power=station,generator,substation,sub_station
  * tourism=zoo
  * military=barracks
* The minimum zoom level of labels is now defined based on the number of pixels
  rendered (resolves partially gravitystorm#703, resolves gravitystorm#861, resolves gravitystorm#913).
  Labels are rendered from 3000 pixels (but never earlier than the corresponding
  landuse is rendered).
* Font size now also depends on way_pixels. In other words, larger objects get
  a larger label, in three steps (resolves gravitystorm#308).
* Landuse labels are now rendered oblique, to easier visually tell them apart
  from village and POI labels.
* All labels are rendered in a colour similar to the landuse they belong to
  (using landuse color variables, resolves gravitystorm#56). Also some existing colours
  are changed, in order to make them clearly colourful but still readable.
* The text-halo-radius and text-wrap-width properties are made consistent across
  landuse types.
* Font-size, wrap-width an face-name are now defined by easy to change
  variables.
matthijsmelissen added a commit to matthijsmelissen/openstreetmap-carto that referenced this issue Sep 22, 2014
This commit changes the rendering of landcover labels.

For the purpose of this commit, with 'landcover label' we mean text connected to
a background colour or pattern rendering, and not connected to an icon.

* All rendered landcover tags now have their name rendered (resolves gravitystorm#537). We
  add labels to the following tags:
  * natural=beach,scrub,grassland,heath,sand,desert (partially resolves gravitystorm#788)
  * highway=services,rest_area (resolves gravitystorm#575)
  * aeroway=apron
  * power=station,generator,substation,sub_station
  * tourism=zoo
  * military=barracks
* The minimum zoom level of labels is now defined based on the number of pixels
  rendered (resolves partially gravitystorm#703, resolves gravitystorm#861, resolves gravitystorm#913).
  Labels are rendered from 3000 pixels (but never earlier than the corresponding
  landuse is rendered).
* Font size now also depends on way_pixels. In other words, larger objects get
  a larger label, in three steps (resolves gravitystorm#308).
* Landuse labels are now rendered oblique, to easier visually tell them apart
  from village and POI labels.
* All labels are rendered in a colour similar to the landuse they belong to
  (using landuse color variables, resolves gravitystorm#56). Also some existing colours
  are changed, in order to make them clearly colourful but still readable.
* The text-halo-radius and text-wrap-width properties are made consistent across
  landuse types.
* Font-size, wrap-width an face-name are now defined by easy to change
  variables.
@matthijsmelissen
Copy link
Collaborator

Resolved now for landcover - but there are still other parts of the code to which we should apply this.

@dieterdreist
Copy link

@gravitystorm

Also, there are problems with using this on adjacent areas. For example, large rivers are often split up into smaller polygons, and many small adjacent polygons might not be matched by an area filter yet the gap is noticeable. We can't force mappers to create large areas out of lots of adjacent small ones. We have similar issues where e.g. forests are split up by roads running through them.

Yes. My idea is to have stuff separated: one (or more) objects to say: here are trees (can be rendered according to the relevant tags, e.g. landuse=forest or landcover=trees). Another object (or in simple cases the same object) to state: this is a named piece of forest (natural=forest/wood with name=*). This second object might also include other areas that do belong to the forest as an entity, but aren't actually forest by their nature (e.g. a lake in the forest, a village in the forest, clearings in the forest), and could be used to render a label with the name (but not the forest polygon itself).

For polygon rendering ("colouring the background") we shouldn't omit small polygons (at least not those bigger than one pixel) because objects can always be split arbitrarily or for some good reason and the result would be gaps.

@matthijsmelissen
Copy link
Collaborator

Using way_area for landcover has now been implemented. Using way_area for buildings is #1121. If it should be used for anything else, feel free to create a new ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants