ogr2osm ignores elevation data of GeoJSON files

[Vectorial1024]

While converting a GeoJSON file to a OSM file using ogr2osm, I noticed that ogr2osm will ignore the elevation data when assigning temporary OSM nodes to the GeoJSON LineStrings. This has created non-existent ways between supposedly-separate nodes.

Example:

GeoJSON CRS: { "type": "name", "properties": { "name": "urn:ogc:def:crs:OGC:1.3:CRS84" } }
GeoJSON Node 1: longitude, latitude, 5; belongs to GeoJSON LineString 1
GeoJSON Node 2: longitude, latitude, 12; belongs to GeoJSON LineString 2
Node 1 and Node 2 belong to different GeoJSON line strings

The above 2 nodes will be considered as the same node by ogr2osm
Effect: LineString 1 and LineString 2 are suddenly connected to each other in the resulting OSM file

This GitHub document also confirms the same thing:

https://github.com/osmandapp/OsmAnd-tools/blob/master/obf-generation/basemap/ogr2osm/ogr2osm.py

An outstanding issue is that elevation in 2.5D features (that can be generated by
reprojecting) is ignored completely.

---

Is this a problem of ogr2osm that should be fixed? Or, is there some other ways for me to mitigate this (eg change the CRS inside the GeoJSON file so that ogr2osm can read the elevation data)?

Thanks in advance!

[roelderickx]

I don't think this is something to be fixed in ogr2osm since the OSM format does not support elevation data. Currently ogr2osm processes the 2.5D features but only takes longitude and latitude in account (or X and Y as they are called in ogr).

That doesn't mean you can't do something creative with the third dimension 🙂 It involves writing some logic in a translation file but I don't think it is really straightforward here. I'll probably have more time in a couple of weeks, if you want to investigate yourself in the meanwhile then the following information might be of interest to you:

• Issue unhandled geometry, type: 3003 pnorman/ogr2osm#39 and the translation file on the OSM wiki page. The translation file is for an older version of ogr2osm and will not work out of the box, but it shows how to intercept specific geometries in filter_feature.
• You'll need the merge_tags translation method to avoid merging the same node with different elevations. For that to work it is necessary to add the elevation as a tag in the filter_feature step above.
• Should you not want the elevation as a tag in the resulting OSM file you can remove it again in process_feature_post or process_output.

[Vectorial1024]

Thanks for the info! I will just read over the texts in the coming days.

[Vectorial1024]

Hmm, it seems we will need to implement this logic.

I reviewed my source GeoJSON file, and it only contains LINESTRINGs, and have no POINTs. Your proposed method may work when I have POINTs inside the GeoJSON, but if I don't have them, then when ogr2osm is reading the LINESTRING and implicitly creating POINTs, there are no existing POINTs to choose from, and the merging will always flatten the elevation.

For my specific case, perhaps I can modify the GeoJSON file to also contain POINTs to make it work.

---

However, I am thinking your method still does not work in general.

No matter what information I store as tags inside the POINTs, once ogr2osm goes to parse e.g. LINESTRINGs, it will

ogr2osm/ogr2osm/osm_data.py

Lines 186 to 187 in f7d2f33

	(x, y, z_unused) = ogrgeometry.GetPoint(i)
	node = self.__add_node(x, y, {}, True)

and also

ogr2osm/ogr2osm/osm_data.py

Lines 91 to 95 in f7d2f33

	unique_node_id = None
	if is_way_member:
	unique_node_id = (rx, ry)
	else:
	unique_node_id = self.translation.get_unique_node_identifier(rx, ry, tags)

ogr2osm/ogr2osm/osm_data.py

Lines 100 to 111 in f7d2f33

	if unique_node_id in self.__unique_node_index:
	for index in self.__unique_node_index[unique_node_id]:
	duplicate_node = self.__nodes[index]
	merged_tags = self.translation.merge_tags('node', duplicate_node.tags, tags)
	if merged_tags is not None:
	duplicate_node.tags = merged_tags
	return duplicate_node
	node = OsmNode(x, y, tags)
	self.__unique_node_index[unique_node_id].append(len(self.__nodes))
	self.__nodes.append(node)
	return node

which results in the following:

• when parsing LINESTRINGs, existing POINTs will always see {} as the "other" tag
• because no existing POINTs can confidently say the given point is "me", the way-member POINTs will once again be added to the node list, which again flattens elevation
• there are now at least 2 POINTs which are unconnected:
  • the POINT from the GeoJSON POINT
  • the POINT implied from the LINESTRING

---

However, if the existing code is already mentioning

(x, y, z_unused) = ogrgeometry.GetPoint(i) 
# z can be loaded but is unused

then perhaps we are very close to implementing the relevant logic.

[roelderickx]

I see, your observations are correct. It is pointless to add the linestring tags to every single node in the resulting osm file, but then only the coordinates can be used to decide if merging is necessary or not.

In order to do something creative with the z-value it should be available, but I would not limit the use to merging or not as you did in your pull request. The logic you want can be implemented by adding the z-value as a tag, ogr2osm will automatically consider two nodes with different tags as 2 separate nodes (which can still be overridden using a translation file if required).

Looking at the pull request you created you already found where to add the logic but the implementation is a bit different.
In __parse_point:

tags.update({ 'elevation': str(ogrgeometry.GetZ()) })
return self.__add_node(ogrgeometry.GetX(), ogrgeometry.GetY(), tags, False)

and in __parse_linestring:

(x, y, z) = ogrgeometry.GetPoint(i)
node = self.__add_node(x, y, { 'elevation': str(z) }, True)

No need to modify anything in __add_node.

Of course this was a quick test, I do not have time at the moment for the full implementation. We need the following:

• a parameter --z-value-tag specifying the name of the tag we want the z-value to have, in my example it is always 'elevation' but it may interfere with other tags in other use-cases
• if the parameter is not specified then no tag should be added
• if the parameter is specified and there is no z-value then no tag should be added either
• a tag with the specified name can exist already, what to do in such a case? Silently overwrite its value? Issue a warning? Quit with an error?

Also, while I was thinking about this issue a question came up. How can two nodes with the same coordinates have two different z-values? It implies the z-value is used for something else than the elevation, which is fine for me but it shows the need to think about a general solution where the z-value is available somehow in the translation file.

[Vectorial1024]

Indeed, my PR is a naive implementation that makes things work when trying to split nodes of different Z elevations.

Perhaps a full implementation would require us to handle user-supplied "get elevation of node" function (and also store the elevation elsewhere in the code) so that we can at least inject the node elevation back to the translation file when e.g. merging nodes, instead of just giving a {} when handling lineestring nodes.

---

Also, something like --elevation-tag-name X to let user specify a non-conflicting elevation tag name would be nice. We can still have a default (e.g. just elevation), but if the user know the list of tag names that may appear and know that it will conflict with our logic, they can just give an override through that switch. Or, we can detect that the name is already in use and stop the whole process.

[roelderickx]

It is implemented as follows:

• A parameter --add-z-value-tag TAGNAME is added
• It seems ogrgeometry.CoordinateDimension() returns 2 if the z-value is 0, no matter if it is specified in the source file or not. Since that does not provide us a reliable way to detect if a point is 2 or 3 dimensional, a tag TAGNAME is added to all nodes with the elevation as value if the parameter above is specified
• If a tag with the same name already exists, the tags are merged using the merge_tags function in the translation. If no translation is specified then the default rules apply.
• Test cases are added

I will give it a few days before merging into the main branch, I usually find another bug when I look at it with fresh eyes. Don't hesitate to test branch issue36 in the meanwhile to see if it meets your requirement.

[roelderickx]

Merged into main, release follows later this week.