Esri / arcgis-osm-editor

ArcGIS Editor for OpenStreetMap is a toolset for GIS users to access and contribute to OpenStreetMap through their Desktop or Server environment.
Apache License 2.0
399 stars 131 forks source link

OSM File Loader (Load only) tool falsely loads elements with osmSupportingElement='yes' #141

Closed mboeringa closed 8 years ago

mboeringa commented 8 years ago

Hi @ThomasEmge,

I have noticed an issue specific to the new OSM File Loader (Load only) tool. The issue may be specific to the processing of buildings, but may also be a more general issue related to multipolygons and their tagging and processing.

What I am seeing is that in cases where the outer way of a multipolygon has the same building=yes tag as the multipolygon itself, that it still gets inserted in the Polygon table, even though this element would be assigned osmSupportingElement=yes had it been loaded with the Load OSM File tool. This leads to an undesirable stacking of a false building contour of the outer way combined with the correct multipolygon.

CORRECTION to the above paragraph: The specific example multipolygon shown here is this one: http://www.openstreetmap.org/relation/357082 It actually turns out this feature does not have a building=yes tag on the multipolygon relation, which in fact has no tags. It just has building=yes on the outer, which, according to the changes related to #72, now get properly transferred to the multipolygon.

However, in the context of the OSM File Loader (Load only) tool, all osmSupportingElement=yes features of the Polygon and PolyLine table should be discarded, as this tool no longer creates the osmSupportingElement field showing the difference between the features, so features can no longer be distinguished or removed by using a Definition Query. In this case this means discarding the Polygon based on the outer way.

@ThomasEmge: Is this issue possibly related (and maybe even solved but not yet released) in issue #117?

The following images show the issue:

The first image shows the outer way with the building=yes tag as created by the OSM File Loader (Load only) tool selected in ArcMap, this feature should have been discarded. Notice it overlaps with the multipolygon which sits on top: osm_file_loader_w45671472_osmsupportingelement_is_yes

The second image shows the relation with building=yes tag as created by the OSM File Loader (Load only) tool selected in ArcMap, this feature is correct: osm_file_loader_r357082_osmsupportingelement_is_no

The third image shows the outer way with ID 45671472 correctly set to osmSupportingElement=yes by the Load OSM File tool: load_osm_file_w45671472_osmsupportingelement_is_yes

The fourth image shows the mulitpolygon with ID 357082 correctly set to osmSupportingElement=no by the Load OSM File tool: load_osm_file_r357082_osmsupportingelement_is_no

ThomasEmge commented 8 years ago

Well, the tool works as designed but I do see the issue of 'overlapping rings' for the building. Deleting the geometry is not really an option as I can't determine at loading time if the geometry (like the outer way) is involved in several other features.

mboeringa commented 8 years ago

Quote by @ThomasEmge:

"Since there is no longer an osmSupportingElement on polygons or lines, everything with an ‘attribute’ will be rendered. Which in this case, because the outer way has a key-value pair of “building=yes” it is rendered. "

I agree there is some ambiguity here, but this is a quite clear case:

I really think that in these cases where tags are transfered, that the multipolygon presentation overrides the outer, and the outer should be removed.

There is no other way, the bad consequence of not removing the outer, is that we now end up with two features having the same tag set. This is really not how it should be. With the Load OSM File tool, this was easy to overcome, as the "osmSupportingElement" field allowed to distinguish the features. Any feature having osmSupportingElement=yes could be hidden using a Definition Query.

This is no longer the case with the new tool. While the absence of the osmSupportingElement field by itself is not bad, the only logical conclusion is that in case tag transfer takes place, the outer should be ditched.

Well, the tool works as designed but I do see the issue of 'overlapping rings' for the building. Deleting the geometry is not really an option as I can't determine at loading time if the geometry (like the outer way) is involved in several other features.

@ThomasEmge In the specific case there is tag transfer, you must be able to determine if the feature is part of a multipolygon, because otherwise you wouldn't have been able to implement #65, and wouldn't have been able to build the multipolygons. Or am I missing something?

mboeringa commented 8 years ago

as I can't determine at loading time if the geometry (like the outer way) is involved in several other features.

Could you delete them at the end of the process, by adding some field keeping track of "involvement with other features". If none, ditch them at the end?

ThomasEmge commented 8 years ago

The tool would be able to tell that the transfer is supposed to happen but it is does require an update to an existing row like setting the osmSupportingElement attribute to true. For the tool is means that I have at least 2 types of cursors open on the feature class and with the new tool bringing it down to one cursor brings a nice performance gain. Let me try some options for tracking "involved features".