Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add explicate coastline (ocean, lake, * water) to tiles #140

Closed
nvkelso opened this issue Aug 19, 2015 · 34 comments
Closed

Add explicate coastline (ocean, lake, * water) to tiles #140

nvkelso opened this issue Aug 19, 2015 · 34 comments
Assignees

Comments

@nvkelso
Copy link
Member

nvkelso commented Aug 19, 2015

We need to stroke the water coastlines directly.

Right now stroking the existing water polygons doesn't work because that adds blue lines at tile boundaries / feature chunks.

@bcamper
Copy link
Collaborator

bcamper commented Aug 20, 2015

To clarify, in Tangram, that should not add lines at tile boundaries because Tangram filters them out (unless explicitly turned on in the stylesheet), however, it does add lines where different water polygons are joined. So yeah, probably necessary for that case.

@rmarianski
Copy link
Member

Natural earth coastline data ok to use here?

  • [0, 4) -> 110m
  • [4, 7) -> 50m
  • [7, *) -> 10m

And should simplification just be turned off completely here for all zoom levels?

@nvkelso
Copy link
Member Author

nvkelso commented Aug 25, 2015

Yes, we shouldn't simply Natural Earth at all (it's pre simplified).

I'm wondering if we should be using the 110m data at all – I think the 50m could be used just as well from [0,5] and 10m from [5,7] and OSM [8,*] for almost all themes (including coastline). Once simplification is turned off, we should verify this hypothesis.

  • For Natural Earth there's an explicate Coastline geometry to source from.
  • For OSM looks like the equivalent is: natural:coastline.

But... this doesn't necessarily solve the problem because waterway:riverbank and natural:coastline still meet at overlaps like: https://www.openstreetmap.org/edit?#map=17/40.74168/-74.12203 in New Jersey.

I wonder if we can take the linestrings from each of OSM sources (coastline, lakeshore & etc), and then take the Symmetric Difference to remove the overlapping parts?

@nvkelso
Copy link
Member Author

nvkelso commented Aug 25, 2015

Other areas like this one in California show that water polygons will overlap sometimes, so we'd need to erase the overlapping bits of the polygons, then do the symmetric difference of their boundary linestring.

https://www.openstreetmap.org/edit#map=17/39.46597/-121.59752

@rmarianski
Copy link
Member

I'm wondering if we should be using the 110m data at all – I think the 50m could be used just as well from [0,5] and 10m from [5,7] and OSM [8,*] for almost all themes (including coastline). Once simplification is turned off, we should verify this hypothesis.

Generally speaking, it's faster to query against the 110m datasets. We can run a test to verify. I think we would also want to align with the intervals that we use in the water/earth queries, which are at the steps that I mentioned above.

Other areas like this one in California show that water polygons will overlap sometimes, so we'd need to erase the overlapping bits of the polygons, then do the symmetric difference of their boundary linestring.

Yes, that issue has come up already in our water layer. Do you think that we'll need to precisely define which features "win" when there's an overlap?

Also, would it be ok if we considered this a separate effort, and made a new issue for it? ie only add the coastlines for now and then tackle the overlapping later?

When you say erase the overlapping bits and then do the symmetric difference with the boundary linestring, do you mean first create water polygons/lines that don't have any overlap, and then ensure that there isn't overlap with the coastline data?

@nvkelso
Copy link
Member Author

nvkelso commented Aug 26, 2015

Generally speaking, it's faster to query against the 110m datasets. We can run a test to verify. I think we would also want to align with the intervals that we use in the water/earth queries, which are at the steps that I mentioned above.

The 1:110m dataset was made for very small print locator "globes". I don't think the 1:50m dataset could be too much slower at those zooms. If it is significantly slower, getting better about invalidating only portions of the cache instead of the whole cache is probably better.

Yes, that issue has come up already in our water layer. Do you think that we'll need to precisely define which features "win" when there's an overlap?

I don't think the order matters for this operation. If it does ocean (coastline) should probably win over river and lake features.

Also, would it be ok if we considered this a separate effort, and made a new issue for it? ie only add the coastlines for now and then tackle the overlapping later?

We can currently stroke the existing polygons in Tangram, and it results in a "coastline" that exhibits the bad lines thru the water at polygon seams (when we just want it on the "edge" of the visual aggregate of all the water polygons.

This issue needs to address both (it doesn't have utility without also tackling the overlap).

When you say erase the overlapping bits and then do the symmetric difference with the boundary linestring, do you mean first create water polygons/lines that don't have any overlap, and then ensure that there isn't overlap with the coastline data?

+------+------+
|   r  |  o   |
+------+------+

Where r = river polygon (waterway:riverbank, natural:water), o = ocean polygon (natural:coastline).

We want to end up with

+------+------+
|             |
+------+------+

Where the interior boundary between r & o has been removed. When the boundary is topologically precise we can use Symmetric Difference alone on the line strings (the outer way shape of the polygon), but if it's not, we need to preprocess it.

@rmarianski
Copy link
Member

The 1:110m dataset was made for very small print locator "globes". I don't think the 1:50m dataset could be too much slower at those zooms. If it is significantly slower, getting better about invalidating only portions of the cache instead of the whole cache is probably better.

If we're querying the 50m dataset for the coastlines layer and the 110m dataset for earth and water, will there be alignment issues?

Where the interior boundary between r & o has been removed. When the boundary is topologically precise we can use Symmetric Difference alone on the line strings (the outer way shape of the polygon), but if it's not, we need to preprocess it.

When the boundary is precise, then yes, symmetric difference works. But I would expect union to be more appropriate for polygons that have overlap, otherwise the difference will produce a hole. And a union has the same behavior in the example you outlined.

But in overlapping polygon scenario, or even the one in your example, what should the results be? The resulting geometry for both (after a union or symmetric difference) would end up being:

+------+------+
|             |
+------+------+
  1. keep both features with their properties, they just have the same exact unioned geometry?
  2. throw out the riverbank feature, and keep the ocean feature with its properties?
  3. throw out the riverbank feature, and copy any properties it may have had to the ocean feature's properties that don't conflict? Or have some sort of merge logic?

@nvkelso
Copy link
Member Author

nvkelso commented Aug 26, 2015

If we're querying the 50m dataset for the coastlines layer and the 110m dataset for earth and water, will there be alignment issues?

Your totally right, we need to be consistent when sourcing data for the zoom. Let's track the general 50m versus 110m change in: #145 separate from the coastline change.

approaches to take

Try a few options and see what is more performant? Ideally the resulting features would still know their original OSM tags.

@nvkelso nvkelso added the ready label Aug 28, 2015
@nvkelso nvkelso changed the title Add explicate coastline to tiles Add explicate coastline (ocean, lake, * water) to tiles Sep 2, 2015
@nvkelso
Copy link
Member Author

nvkelso commented Sep 2, 2015

From the old Trello board (Rob):

Right now some water area polygons (rivers, etc.) overlap ocean polygons, causing duplicate/overlapping polygons to be returned. This can cause rendering issues, in addition to just being wasteful (and potentially confusing) to data consumers.

Ideally we would have no overlaps. One way to accomplish this would be to "cut out" the intersecting areas from the ocean polygons as a pre-processing step; since the area polygons are generally more specific, they should usually take precedence.

@zerebubuth
Copy link
Member

With tilezen/tilequeue#25 and tilezen/TileStache#43 and #185, we get this (to compare with the screenshot in #175):

screenshot from 2015-09-10 15 45 09

Which probably also explains why I'm not a cartographer 😉

@nvkelso could you review, please?

@bcamper
Copy link
Collaborator

bcamper commented Sep 10, 2015

Nice.

I see a little seam in the middle of the water there, is that a known issue?

On Thu, Sep 10, 2015 at 11:03 AM, Matt Amos notifications@github.com
wrote:

With tilezen/tilequeue#25 tilezen/tilequeue#25
and tilezen/TileStache#43 tilezen/TileStache#43
and #185 #185, we get
this:

[image: screenshot from 2015-09-10 15 45 09]
https://cloud.githubusercontent.com/assets/271360/9791915/5f5505be-57d5-11e5-8045-36973b82d16b.png

Which probably also explains why I'm not a cartographer [image: 😉]

@nvkelso https://github.com/nvkelso could you review, please?


Reply to this email directly or view it on GitHub
#140 (comment)
.

@zerebubuth
Copy link
Member

@bcamper: Thanks!

As for the seam, there's one triangular bit of the river with coords

0 144
378 0
0 0
0 144

and another bit which ends

...
658 0
381 0
0 145
0 850

The boundary at that point isn't quite straight (see way 135719690), although it takes a ruler to see it. Is it possible that the two polygons are being simplified slightly differently each side of the boundary?

@bcamper
Copy link
Collaborator

bcamper commented Sep 10, 2015

Hmm yes, simplification does seem a likely culprit.

@nvkelso
Copy link
Member Author

nvkelso commented Sep 10, 2015

We can try hide bad data seams by stroking the water polygon with the same color as the fill, and then drawing the new coastline layer over that.

@zerebubuth
Copy link
Member

Yup, that's a work-around. We could also try lowering the simplification tolerance.

The real fix is to not throw away the topology in OSM and use it while simplifying to ensure polygons sharing a way / series of nodes at the boundary simplify to the same shape. (This could also help when outputting topojson, as we'd be able to share more). But fixing this properly would mean pretty epic changes to the stack.

@bcamper
Copy link
Collaborator

bcamper commented Sep 10, 2015

@nvkelso that's a lot of extra line geometry as a band-aid :/ (remember lines have to be tessellated into triangles, could be as many extra triangles as the polygon itself). If there are any simpler server-side improvements we can try, I think it's worth it. What I've done instead of lines is to just rely on map background color to fill in (only works for water obviously, and will show through if there is a pattern or other lighting on the water).

@nvkelso nvkelso modified the milestone: Compilation 2.0 Sep 10, 2015
@nvkelso
Copy link
Member Author

nvkelso commented Sep 10, 2015

Let's investigate the water seam rips with: #191

@nvkelso
Copy link
Member Author

nvkelso commented Sep 10, 2015

Rob's reviewed the PR for this and they've been merged.

Question: Should the result of this operation go into the existing water layer (looks like it's going into a new water-boundaries layer). @bcamper do you have an opinion? For instance, we're planning to shove addresses into the building layer.

@zerebubuth
Copy link
Member

Yup, but I didn't want to close out this in case we weren't finished.

I reckon it makes more sense to have the boundaries in separate layer, especially since the geometry type expected is different. I would have thought we'd have to do further gymnastics in the scene file to differentiate river lines from riverbank lines if we conflated them in the same layer.

@bcamper
Copy link
Collaborator

bcamper commented Sep 10, 2015

Hmm... I see pros/cons with either same or new layer. New layer is probably safest (though the proliferation of layers bothers me slightly)? As far as scene file, we can filter by both properties and geometry types, so if there the necessary properties to distinguish them are present, we can do it in the same layer. Either works!

@nvkelso
Copy link
Member Author

nvkelso commented Sep 11, 2015

@zerebubuth what is the new line's kind? Where in the code is that being set?

Styling by layer is generally bad practice and I'm generally in favor of less layers. Let's shove it in the existing water layer, with (a) distinct kind(s).

@rmarianski
Copy link
Member

@nvkelso at the moment it's just a copy of the corresponding feature's properties from the water layer, kind and all others. I think this is the relevant code (note the props.copy() on line 758):

https://github.com/mapzen/TileStache/blob/de67a59d1debfd25e7ab082689b6c7f9263515f3/TileStache/Goodies/VecTiles/transform.py#L749-L758

Would it be preferable to just add the new features to the existing water layer with a new kind?

@nvkelso
Copy link
Member Author

nvkelso commented Sep 11, 2015

Yes, we should add the new edge features to water layer but with new kind values.

Should those kinds follow an OSM naming convention like coastline for ocean edges and riverbank for river polygon edged and shoreline for lake edges? (Please sub with the correct names, I'm making them up from memory.) Are there any other kinds of water edges in our tiles?

@zerebubuth
Copy link
Member

After #196, looks like this:

water_boundaries_kinds

Note the "dashing" on the tributary to the right. That's caused by multiple, partially overlapping, polygons. I tried buffering the polygon a little, but that's actually not very helpful as it removes the line entirely. Probably the right way to fix it is to fix the original datasource, as one of these polygons is riverbank and the other is ocean.

The scene.yaml fragment used to render this is:

    water:
        data: { source: osm }
        draw:
            polygons:
        order: 2
                color: '#88bbee'
        water_boundaries:
          filter: { boundary: yes }
          draw:
            lines:
              order: 3
              color: '#ff0000'
              width: 20
              cap: round
          shoreline:
            filter: { kind: shoreline }
            draw: { lines: { color: '#3030a0' } }
          riverbank:
            filter: { kind: riverbank }
            draw: { lines: { color: '#6060d0' } }
          dock:
            filter: { kind: dock }
            draw: { lines: { color: '#3080a0' } }
          water:
            filter: { kind: water }
            draw: { lines: { color: '#4040b0' } }
          basin:
            filter: { kind: basin }
            draw: { lines: { color: '#1050d0' } }
          reservoir:
            filter: { kind: reservoir }
            draw: { lines: { color: '#5020e0' } }

@zerebubuth zerebubuth assigned nvkelso and unassigned zerebubuth Sep 11, 2015
@nvkelso
Copy link
Member Author

nvkelso commented Sep 11, 2015

@zerebubuth Where are the water boundary kind's coming from? Are they using an OSM convention or?

See also:

@nvkelso nvkelso assigned zerebubuth and unassigned nvkelso Sep 11, 2015
@nvkelso
Copy link
Member Author

nvkelso commented Sep 11, 2015

The original kind to new kind lookup is here: https://github.com/mapzen/vector-datasource/pull/196/files

@zerebubuth
Copy link
Member

Yup, that's where the mapping is. There aren't really original OSM values for these, except that we'll want to avoid "shoreline" and "riverbank" as those might already exist and refer to other things. Let me know what mapping you want and I'll put it in there.

@nvkelso
Copy link
Member Author

nvkelso commented Sep 11, 2015

Regarding the dashing, I think that's just a fact of life with the data (if someone sees this bug, they should fix the data). We should probably avoid buffering the geoms unless we know the output will match the input for 95%+ of the so the TopoJSON file sizes don't go too crazy.

@zerebubuth
Copy link
Member

Agreed - the buffering didn't help anyway.

@nvkelso
Copy link
Member Author

nvkelso commented Sep 11, 2015

For the property names, are these the options?

  1. Add a new boundary=yes property, and leave the kind=* property as-is.
  2. Add a new boundary=yes property, and transform some of the existing kind=* properties (like ocean goes to coastline).
  3. Add a new boundary=yes property, and transform all of the existing kind=* properties.

@zerebubuth
Copy link
Member

We don't have to add boundary=yes, that just seemed like the easiest way to distinguish between the boundary and the polygon for writing the example scene file. In general, we can:

  1. Add any new property with a fixed value.
  2. Copy any property from the original feature.
  3. Copy a property from the original feature, remapping its value.

And we can do multiple ones of these, see https://github.com/mapzen/TileStache/blob/140-add-explicit-water-boundaries/TileStache/Goodies/VecTiles/transform.py#L651-L659 for the algorithm that's proposed.

@nvkelso
Copy link
Member Author

nvkelso commented Sep 11, 2015

Let's go with my #1 (combo of your #1 and #2):

Add a new boundary=yes property, and leave the kind=* property as-is (copy it).

Everything else looks good in the PR. I'd leave the property substitution function in there, it'll come in useful sooner than later.

@zerebubuth
Copy link
Member

Leaving the kind as-is in edb7959.

@nvkelso
Copy link
Member Author

nvkelso commented Sep 11, 2015

#196 has been merged, closing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants