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

fill may render on wrong side of incomplete multipolygons #1201

Open
tmcw opened this issue Mar 29, 2013 · 15 comments
Open

fill may render on wrong side of incomplete multipolygons #1201

tmcw opened this issue Mar 29, 2013 · 15 comments
Labels
bluesky Bluesky issues are extra challenging - this might take a while or be impossible bug A bug - let's fix this! data An issue with the data built into iD map-renderer An issue with how things are rendered in the map

Comments

@tmcw
Copy link
Contributor

tmcw commented Mar 29, 2013

This way/relation has the tag waterway=polygon and renders incorrectly

@jfirebaugh
Copy link
Member

It is an area, it's just that the relation is incomplete (hasn't been fully downloaded). It might be better not to render a fill for incomplete relations (although JOSM does).

image

@pnorman
Copy link
Contributor

pnorman commented Sep 23, 2013

Copied from #1832:

The options are

  1. Don't render the fill at all
  2. Issue a /relation/id#/full query for incomplete relations after all map? queries have finished. This is equivalent to doing JOSM's "Download Members" for each incomplete relation.
  3. Issue one or more /ways?ways=... queries for the relation followed by /nodes?nodes=... for the missing nodes. This is equivalent to doing JOSM's "Download Incomplete Members" for each incomplete relation.
  4. Defer this until we have a proper area type.

2 and 3 are largely the same. Some issues with them

  • Editing near a country border will issue a massive number of API calls. These should be type=boundary not type=multipolygon so maybe we can restrict to that to avoid having to the big fetch of very large relations
  • I have no idea what the load impact is for doing this is, although I can come up with an informed opinion if necessary
  • The /relation/id#/full call has been known to time out, but is now supported by cgimap (Move full calls out of experimental zerebubuth/openstreetmap-cgimap#51). For a relation where most of the children are not known locally,it's more efficient, but if you've got most of the relation downloaded, individually downloading the remaining members is better.

@pnorman
Copy link
Contributor

pnorman commented Sep 26, 2013

I talked with @zerebubuth and others and the suggestion was to use /relation/id#/full when all the map? queries are done to load relations that are both incomplete and of type=multipolygon. This query should return quickly for small MPs. For large MPs there will be a period where the current bugs are still present, but there's no way around that.

Also, that period where the current bugs are present only occurs on the first load, when scrolling around the MP will no longer be incomplete.

It's more load on the server, but that's a consequence of the data model. The data is necessary to be able to properly edit in the area.

@bryceco
Copy link
Contributor

bryceco commented Aug 25, 2014

Wouldn't honoring the "water is on the right side" rule address many of these rendering errors?

@pnorman
Copy link
Contributor

pnorman commented Aug 25, 2014

Wouldn't honoring the "water is on the right side" rule address many of these rendering errors?

That's for coastlines, not multipolygons.

@bryceco
Copy link
Contributor

bryceco commented Aug 25, 2014

On Aug 24, 2014, at 11:06 PM, Paul Norman [email protected] wrote:

Wouldn't honoring the "water is on the right side" rule address many of these rendering errors?

That's for coastlines, not multipolygons.


Reply to this email directly or view it on GitHub.

I mean as a heuristic when the alternative is guessing. It works for the multipolygon natural=water features in my area where they were originally tagged as coastlines and later converted to water.

@pnorman
Copy link
Contributor

pnorman commented Aug 25, 2014

Guessing based on area seems like it would confuse people as it means rendering will change with a way reversal even if it is semantically a non-operation.

It'd be nice to know if doing it the right way as suggested in #1201 (comment) would work.

@slhh
Copy link
Contributor

slhh commented Feb 12, 2016

The rendering issue of broken or incompletely loaded multipolygons could be addressed in the following way: Define additional roles like outer:right and outer:left indicating on which side of the way the multipolygon area is located. The user should not need to add these suffices, but the editor can automatically add or update these, if the multipolygon is complete. This would significantly reduce the impact of broken multipolygons.

Being a change of the OSM datastructures this needs to be discussed with the community in case the change is desired.

@bhousel
Copy link
Member

bhousel commented Feb 16, 2016

I guess much has been written about this issue since 2013. see: The Future of Areas for a current summary.

So where things stand right now, iD will always wrongly render certain multipolygons unless we download the entire multipolygon. Some multipolygons are too prohibitively large to download.

@slhh's solution of adding tags to indicate which side of the way contains the area could work. (It sounds a bit like Tagging Outlines from the above link, but not exactly). It would definitely upset a lot of people in the OSM community if iD started silently tagging multipolygon member ways with this extra data.

That said, we can use Any Tags We Like. I'm kind of inclined to push OSM in this direction and just start doing it. Thoughts @pnorman?

@pnorman
Copy link
Contributor

pnorman commented Feb 17, 2016

That said, we can use Any Tags We Like. I'm kind of inclined to push OSM in this direction and just start doing it. Thoughts @pnorman?

Don't.

So where things stand right now, iD will always wrongly render certain multipolygons unless we download the entire multipolygon. Some multipolygons are too prohibitively large to download.

I haven't found this an issue in practice. Do you have data which indicates otherwise?

I'd still recommend /relation/id#/full. I can probably get some solid data on it, but my instinct is that most type=multipolygon relations will be entirely within the downloaded area, and of those that aren't, most will be small.

@bhousel
Copy link
Member

bhousel commented Feb 17, 2016

I haven't found this an issue in practice. Do you have data which indicates otherwise?

I'd still recommend /relation/id#/full. I can probably get some solid data on it, but my instinct is that most type=multipolygon relations will be entirely within the downloaded area, and of those that aren't, most will be small.

@pnorman I went back and read your comment a bit more carefully.

I originally thought we would need to do this background relation/id/full fetch for all relations, but if we limit it to only multipolygons it would probably be ok. Coastlines and boundaries are where this approach wouldn't scale.

@pnorman
Copy link
Contributor

pnorman commented Feb 17, 2016

Coastlines and boundaries are where this approach wouldn't scale.

Yes, boundaries could be large, and take several seconds. Coastlines don't have relations.

I did some numbers, and of the 2 million MP relations

members   number
1           86k
2         1000k
3          260k
4          230k
5          101k
>5         362k

@bryceco
Copy link
Contributor

bryceco commented Feb 17, 2016

The total number of nodes in the MP is more interesting to me than the number of members. That's a better measure of bandwidth and memory cost.

@bhousel
Copy link
Member

bhousel commented Feb 17, 2016

The total number of nodes in the MP is more interesting to me than the number of members. That's a better measure of bandwidth and memory cost.

Yes, but that number is capped at 2000 nodes per member way. It's probably ok.

I don't have time this week, but sometime soon I'll try automatically fetching the multipolygons with relation/id/full. My home state of New Jersey is a great place to test this - it is covered with huge imported landuse multipolygons.

@bhousel
Copy link
Member

bhousel commented Feb 19, 2016

So, @pnorman maybe the more interesting question to ask is how many members these multipolygons max out at. ">5" doesn't sound so scary, but if the max is thousands then we have a problem.

Here's an outlier mentioned in IRC: https://www.openstreetmap.org/relation/5989495
We should test how long that takes to download via relation/id/full

@bhousel bhousel added bluesky Bluesky issues are extra challenging - this might take a while or be impossible data An issue with the data built into iD labels Dec 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bluesky Bluesky issues are extra challenging - this might take a while or be impossible bug A bug - let's fix this! data An issue with the data built into iD map-renderer An issue with how things are rendered in the map
Projects
None yet
Development

No branches or pull requests

7 participants