gravitystorm / openstreetmap-carto

A general-purpose OpenStreetMap mapnik style, in CartoCSS
Other
1.53k stars 818 forks source link

The use of way_area #703

Closed Rovastar closed 9 years ago

Rovastar commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

@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 commented 10 years ago

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 commented 10 years ago

There are no plans to remove these only when they cause problems. like: https://github.com/gravitystorm/openstreetmap-carto/issues/73 Often these will be for labels. The general feel of the OSM style for things nature features will remain unchanged.

vincentdephily commented 10 years ago

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 :

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 commented 10 years ago
  • 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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

I see, thanks.

Rovastar commented 10 years ago

@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 commented 10 years ago

@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 commented 10 years ago

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

pnorman commented 10 years ago

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 commented 10 years ago

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

Nope, that doesn't prevent the bug.

Rovastar commented 10 years ago

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

matthijsmelissen commented 10 years ago

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 commented 10 years ago

And another strange bug in Carto: https://github.com/mapbox/carto/issues/370

matthijsmelissen commented 10 years ago

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

dieterdreist commented 9 years ago

@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 commented 9 years ago

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.