Add vector rendering support for bathymetry
ClosedPublic

Authored by tandon on Jul 19 2016, 8:00 PM.

Diff Detail

Repository
R34 Marble
Lint
Lint Skipped
Unit
Unit Tests Skipped
tandon updated this revision to Diff 5329.Jul 19 2016, 8:00 PM
tandon retitled this revision from to Add vector rendering support for bathymetry.
tandon updated this object.
tandon edited the test plan for this revision. (Show Details)
tandon added reviewers: nienhueser, rahn.
tandon set the repository for this revision to R34 Marble.
tandon added a project: Marble.
tandon added a subscriber: Marble.
rahn edited edge metadata.Jul 19 2016, 8:03 PM

Nice :-) Looks good to go!
Any screenshots for the whole globe? :)

In D2228#41306, @rahn wrote:

Nice :-) Looks good to go!
Any screenshots for the whole globe? :)

Here they are :)

Also, bathymetries start appearing at level 7 onwards( I have set the minimum level as 7 for them) and hence for taking the above screenshots I had to change their min level to 1. Should I change it to a lower level so that they start appearing at earlier levels ?

This revision was automatically updated to reflect the committed changes.

I eventually made the min zoom level for bathymetries to be 1 instead of 7, since the globe appeared better that way.

rahn added a comment.Jul 19 2016, 8:47 PM

Looks great - but makes me realize some potential for improvement:

  • Maybe we could use the Bathymetry -200m as a temporary approximation of the usual ocean (using the normal NaturalWater color)
  • And maybe we should use a different bathymetry level like 3000 or 4000 instead of 1000 which is more distant from the normal water area.
tandon updated this revision to Diff 5340.EditedJul 19 2016, 11:35 PM
tandon edited edge metadata.

Add support for bathymetry levels 200 and 4000 as suggested by @rahn

tandon added a comment.EditedJul 19 2016, 11:47 PM

For the above diff (Diff 5340), I introduced 2 GeoDataFeatures - Bathymetry200 and Bathymetry4000 instead of a single feature. It is because level 4000 bathymetry must always be rendered above the level 200 one.
@nienhueser Is there any way of doing this via a single GeoDataFeature type and StyleBuilder class ?

Screenshots

Bathymetries 200 and 4000



tandon reopened this revision.Jul 19 2016, 11:52 PM

Reopened since additional improvements are required

nienhueser added inline comments.Jul 20 2016, 4:39 AM
tools/shp2osm/polyshp2osm.py
277

We could use a generic bathymetry tag here and an ele one with the depth value. See http://wiki.openstreetmap.org/wiki/Key:ele

Then in Marble map that to one bathymetry visual category and a height value of the polygon, and finally have StyleBuilder adjust the style color based on the height.

tandon updated this revision to Diff 5373.Jul 21 2016, 12:57 AM

This diff uses a single Bathymetry GeoDataFeature and renders different types of bathymetries(currently 200m and 4000m) using the elevation info.

nienhueser edited edge metadata.Jul 21 2016, 3:35 AM

Looks good :-)

src/plugins/runner/osm/OsmRelation.cpp
120 ↗(On Diff #5373)

That seems like a hack that might be problematic when writing the document to a file. Why not just add the ele tag to the placemark?

tandon updated this revision to Diff 5398.Jul 21 2016, 6:49 PM
tandon edited edge metadata.
tandon marked an inline comment as done.

Fixes
Elevation info is now stored in placemark's osmdata instead of outer ring's osm data.

nienhueser accepted this revision.Jul 21 2016, 7:41 PM
nienhueser edited edge metadata.

Looks good, please push.
Seems I am to blame for the need of the z value hack by suggesting to have just one bathymetry category. Maybe we find an easier approach when extending StyleBuilder in the future.

This revision is now accepted and ready to land.Jul 21 2016, 7:41 PM
This revision was automatically updated to reflect the committed changes.