Openstreetmap-carto: natural=cliff is ignored on closed ways

[xsuchy]

See this path of natural=cliff:
https://www.openstreetmap.org/way/547951335
which has the same nodes as area:
https://www.openstreetmap.org/way/563113425

The bare_rock have been rendered, but the cliff tag is then ignored.

[HolgerJeromin]

I dont think the bare rock line (sharing the nodes) is the problem here. This is kind of a duplicate of closed/solved(?) #268
https://www.openstreetmap.org/way/207662049 is 5 years old and not rendered right.
Looks like a regression...

[xsuchy]

Previously there have been just the cliff. Yesterday it was rendered. Then I added the bare_rock and once the rock have been rendered then the cliff disappeared.

[mboeringa]

Previously there have been just the cliff. Yesterday it was rendered. Then I added the bare_rock and once the rock have been rendered then the cliff disappeared.

@xsuchy

Cliff is explicitly defined as a linear feature in the Wiki, and styled as such. The feature you tagged is a closed way. In these cases, to force interpretation as linear way, it is strongly advisable to always add area=no to the feature to resolve the line/polygon ambiguity.

I have now done this for you. The cliff now renders. I did a second edit after I discovered the direction of the way was inappropriate for the cliff rendering (did not conform with Wiki that states right side of way is the low side), so I reverted the direction in a second edit.

The cliff should start rendering fine now.

[HolgerJeromin]

Cliff is explicitly defined as a linear feature in the Wiki, and styled as such. The feature you tagged is a closed way.

The lua transform should make it a linear feature even if it is a closed way

[xsuchy]

@mboeringa Neat. Thank you.

[kocio-pl]

The lua transform should make it a linear feature even if it is a closed way

Could you prepare a PR and test it? We didn't need to change database format yet after v4.0.0 release, but it sounds like something that should be done one day.

[HolgerJeromin]

This is no new idea but something we did already in the current lua.
#2128

[pnorman]

https://github.com/gravitystorm/openstreetmap-carto/blob/12ad3a12c9ca98c3bab454fdff8419d0a7bb04f8/openstreetmap-carto.lua#L34-L43

natural=cliff is already treated as a linestring. adding area=no does nothing to it.

[HolgerJeromin]

@pnorman can you explain the wrong rendering of https://www.openstreetmap.org/way/207662049 ?

[mboeringa]

natural=cliff is already treated as a linestring. adding area=no does nothing to it.

Maybe not in lua, but clearly osm2pgsql did take the area=no into account, as it started rendering immediately after I added the tag.

Initially, the cliff rendering was wrong because of the way direction, but that did not change the fact that it showed up as soon as area=no was added to the object. Note that I only modified the existing feature in OSM by adding the tag, I did not re-draw it.

Reverting the direction just solved the second issue of the incorrect high/low side of the cliff, that did not conform to the Wiki definition.

[mboeringa]

@pnorman can you explain the wrong rendering of https://www.openstreetmap.org/way/207662049 ?

If the error is somewhere in lua, it must be in this part of the code, as this determines what is a polygon:

--- Check if an object with given tags should be treated as polygon
-- @param tags OSM tags
-- @return 1 if area, 0 if linear
function isarea (tags)
    -- Treat objects tagged as area=yes polygon, other area as no
    if tags["area"] then
        return tags["area"] == "yes" and 1 or 0
    end

   -- Search through object's tags
    for k, v in pairs(tags) do
        -- Check if it has a polygon key and not a linestring override, or a polygon k=v
        for _, ptag in ipairs(polygon_keys) do
            if k == ptag and not (linestring_values[k] and linestring_values[k][v]) then
                return 1
            end
        end

        if (polygon_values[k] and polygon_values[k][v]) then
            return 1
        end
    end
    return 0
end

I am slightly puzzled by some parts of the code. I am not to familiar with the specific syntax of e.g.

_" return tags["area"] == "yes" and 1 or 0"_

I find this a bit puzzling statement, especially the _"and 1 or 0"_.

Also, couldn't this statement:

linestring_values[k] and linestring_values[k][v]

with dictionary lookups be shortened to?:

linestring_values[k][v]

Given the more strict collection of _"linestring_values[k][v]"_, the _"linestring_values[k]"_ has no function as it is combined with the and operator? Or am I missing something?

[HolgerJeromin]

linestring_values[k] and linestring_values[k][v]
with dictionary lookups be shortened to?:
linestring_values[k][v]

If linestring_values[k] is not existent in Javascript this would give an exception. Cannot access v of undefined. It think this here is comparable.

[pnorman]

I find this a bit puzzling statement, especially the "and 1 or 0".

area must be a 0 or 1 for osm2pgsql, not boolean.

If linestring_values[k] is not existent in Javascript this would give an exception. Cannot access v of undefined. It think this here is comparable.

Yes, linestring_values[k] might be nil.

If there are bugs, please submit a PR adding them to the test suite in scripts/lua

[mboeringa]

I find this a bit puzzling statement, especially the "and 1 or 0".

area must be a 0 or 1 for osm2pgsql, not boolean.

If linestring_values[k] is not existent in Javascript this would give an exception. Cannot access v of undefined. It think this here is comparable.

Yes, linestring_values[k] might be nil.

If there are bugs, please submit a PR adding them to the test suite in scripts/lua

Thanks for explaining. I was not suggesting that there is necessarily a bug in these specific pieces of code, just that I was puzzled by the unfamiliar syntax, as I have no experience with lua.

[pnorman]

Testing with Lua

o$ lua
Lua 5.2.4  Copyright (C) 1994-2015 Lua.org, PUC-Rio
> require("openstreetmap-carto")
> print(isarea{natural="cliff"})
1
> print(isarea{natural="cliff", area="no"})
0
> print(isarea{highway="service"})
0
> print(isarea{natural="ridge"})
0

I noticed the problem, and once you spot it, it's obvious: the natural key appears twice in the linestring_values collection. So the problem isn't isarea logic, it's the polygon and linestring definitions at the top. This makes it easy to fix.

[kocio-pl]

Looks like OSMF stuff reloaded the database lately:

https://www.openstreetmap.org/way/434006421