[BUG] canBeLine returns true for a closed way

[HarelM]

Describe the bug
canBeLine returns true for a closed way

To Reproduce
Run planetiler on israel and check the return value of canBeLine for the following way: https://www.openstreetmap.org/way/123464650 - which is a closed way.
According to the docs it should return false, shouldn't it?
See here:

planetiler/planetiler-core/src/main/java/com/onthegomap/planetiler/reader/WithGeometryType.java

Line 30 in 4ecb02d

* A closed ring can either be a polygon or linestring, so return false to not allow this closed ring to be treated as

Expected behavior
The sourceFeature.canBeLine() should return false or update the docs to reflect that.

Additional context
I can use f.canBeLine() && f.!canBePolygon() to work around this issue, but I'm wondering this is a bug in the code or should I open a PR to update the doc comment...?

[msbarry]

Planetiler uses the area tag to determine if a closed way can be a polygon or line. area=no means it can only be a line, area=yes means it can only be a polygon, and no area tag means it can either be a line or a polygon. It looks like you need to set area=yes on that way to indicate it shouldn't be interpreted as a line

[HarelM]

Thanks for the quick response!
I'm 100% sure this is a mapping issue as the linked page does say:

This tag is seldom necessary

I don't mind checking in my code both f.canBeLine() && f.!canBePolygon() to make sure something is a line and not an area, but I did find the documentation confusing as I assumed it did some basic geometry checks like making sure the first and last node are the same or something similar...

[HarelM]

It seems that is checks for closed ways as well in the code, not just the area tag:

planetiler/planetiler-core/src/main/java/com/onthegomap/planetiler/reader/osm/OsmReader.java

Line 509 in 4ecb02d

boolean closed = nodes.size() > 1 && nodes.get(0) == nodes.get(nodes.size() - 1);

It might be possible that this comparison should be id based instead of reference based, or am I reading this wrong?

[HarelM]

I probably read the following code wrong:

planetiler/planetiler-core/src/main/java/com/onthegomap/planetiler/reader/osm/OsmReader.java

Line 716 in 4ecb02d

return (!closed || !"yes".equals(area)) && points >= 2;

If I'm reading this correctly, it means it gives precedence to a way without an area tag instead of relaying on the closed value?
I'm probably reading this wrong...

[HarelM]

Here's my suggestion, what do you think - this gives precedence to the area tag and if it's not defined it checks the geometry, another solution would be to give precedence to the geometry first, after that to the area tag.

Method	closed?	area	current outcome	expected outcome
canBeLine	true	yes	false	false
canBeLine	false	yes	true	false*
canBeLine	true	no	true	true
canBeLine	false	no	true	true
canBeLine	true	-	true	false*
canBeLine	false	-	true	true

[msbarry]

I came out with something similar:

	closed	not closed
area=yes	polygon	line
area=no	line	line
area missing/other	polygon or line	line

If we make canBeLine return false when closed and area is missing, or when not closed and area=yes then you will be unable to construct a line feature from those shapes. The current values give more flexibility to be able to do that, but make it harder to find what is most likely a line...

Maybe what we need is:

• isPolygon: closed and area != no
• isLine: !isPolygon
• canBePolygon: closed
• canBeLine: always true?

Then you could always force a line feature to be emitted, even if it is the perimeter of an actual polygon, or force a polygon to be emitted as long as its closed but the is* methods represent the most likely intent of the shape?

[HarelM]

A closed ring can either be a polygon or linestring, so return false to not allow this closed ring to be treated...

I'm good with keeping this idea - meaning when area is not defined, use the value of closed. Otherwise, this comment needs to change as well, doesn't it?

[zstadler]

If area is not defined, and canBeLine would be taken from isClosed, then circular roads will have canBeLine == false.

[msbarry]

I found there are 2 other projects that use other tags in addition to area to indicate what the most likely intent of a close way without an area tag should be:

• id-area-keys - used by iD
• osm-polygon-features - used by Overpass Turbo

The current behavior is that the client will filter these based on tags first, then use line or polygon to make a new one, so they wouldn't currently be missing anything and adding handling for those other tags would further limit what profiles could turn into a line or polygon.

[HarelM]

The following project went a step further in terms of trying to take tags into consideration (somewhat like osm-polygon-features):
https://github.com/OsmSharp/core/blob/b7c5b34eac164850686cadbe9cef47a4a2e4e963/src/OsmSharp.Geo/DefaultFeatureInterpreter.cs#L75

I prefer the simplistic approach of taking into consideration just the geometry itself:
https://github.com/IsraelHikingMap/Site/blob/de911cb386f7279cb2c489b84602440470768da8/IsraelHiking.API/Converters/OsmGeoJsonConverter.cs#L57
https://github.com/IsraelHikingMap/Site/blob/de911cb386f7279cb2c489b84602440470768da8/IsraelHiking.API/Converters/OsmGeoJsonConverter.cs#L286

I did take some tags into consideration for multi polygons though...
Bottom line, it's a hard problem...

Regardless, I ended up changing my code to only check canBePolygon since I wanted to know if I should take a center point for stuff that can be a polygon or the first coordinate for lines and points.

Having said that, I still believe the comment and the code should match, I'm fine either way, let me know if there's anything I can do to help here.

[msbarry]

OK thanks for the research and discussion, sounds like the path forward here should be:

1. keep current behavior and fix comments on those methods to be more clear
2. expose whether the feature is a closed way to profiles
3. some nice-to-haves in the future:
   a. a way to coerce any way to a linestring, or any closed way to a polygon
   b. a utility to that uses tags in addition to just area to tell the profile the most-likely intent of a closed OSM way

?

[HarelM]

Sounds like a plan! Thanks for looking into this!